Quote:
|
Originally Posted by arch13
Ratbastid, I don't have a database to use. I have mysql on the hosting package, but have never even set it up as that's an additonal layer of learning required, and I seem to be trying very little the needs this kind of stuff.
|
Too be honest, a database is the most elegant solution for this kind of thing, but if I'm sensing the correct level of programming, then it's a little ways to go in terms of learning for you. But don't worry, once you get this script down, and you see how databases work sometime, then you'll re-write it in no time
As for the code, no it's not completely correct.
You are indeed using $_REQUEST['id2'] as you suggested (by calling it with $id), but not in the correct way (as it is now, your code would be looking for '1.jpg' and not 'img/brewery_1.jpg').
The code as (I think) it should be is this:
(I've tried to use as much of your code as possible to help you understand what's going on. I also added comments, but if it's not completely clear, please do ask! Check this code against yours above to see the differences, I couldn't find a good way to display the differences...)
Code:
/* Set up external variables */
$id = $_REQUEST['id'];
$id2 = $_REQUEST['id2'];
/* This is the place for the "optional" sanitizing of the variables you got from the URL */
/* Set up constants */
// Note, we're not using constants much, but it helps getting used to them
define('IMAGE_PATH', 'img/'); // From now on we can use IMAGE_PATH, and it will substitute the string 'img/' for it. If you ever change the images directory, then changing this constant is enough for your entire code.
/* setup complicated variables */
$filename=$id.'_'.$id2.'.jpg' // Since we defined $id / $id2 to be the URL variables before, we can use them. Saves typing, and makes it easier to read.
$image_location=IMAGE_PATH.$filename // Becomes 'img/brewery_1.jpg' in our example
/* program flow */
if(file_exists($image_location)) { // We already "built" the location variable, no need to do it again ;)
echo "<img src='$image_location'>"; // Same here...
} elseif(file_exists($id.".php")) {
include($id.".php");
} else {
echo "<img src='brewerysplash.jpg'>";
}
Notes:
- When naming variables that represent values you got from the URL, be careful. Your code was confusing in that you set $id to be URL value id2
- In that respect, to help read your code, it might help to name the variables "gallery" and "img_id" just so it is never unclear of which is which. Of course, it is not necessary, and for the sake of continuing this thread it might be better to leave it as-is for now.
- Seperating the creation of the image path, instead of just using file_exists('img/'.$id.'.jpg) allows for easy manipulation in the beginning of the code while ensuring that any changes propagate nicely. Otherwise a decision to use, say, GIFs would require you to look up all references to .jpg and change them. In the code above only one line of code needs to be changed.
- On that note, it's probably more elegant to have another variable "alternative_location" (or something like that) to define what file should be included if the image is not found (instead of "building" that in your program flow itself.
- Once you get this down, and made it work (Code never works out-of-the-box
)Try looking into sanitizing the input variables ($id / $id2) to make sure that no dots and slashes are in them. Better safe than sorry, and it's good practice.