nvee Posted November 16, 2009 Share Posted November 16, 2009 Hi Guys I have a form with 8 image uploads. Each form input is named img1,img2,img3 .... I also have a function which uploads the file, give it a random new name and then saves it to the server. It uploads 1 file, so the it works, but does not upload the rest. Here is my code: Please note that I have NEVER worked with file uploads prior to today, so I am quite happy that it is atleast uploading one file form: (one element, rest is named and id'd img2, img3 ect <td><input type="file" name="img1" id="img1" /></td> defining the variables and calling the function: $img1 = $_FILES["img1"]; $img2 = $_FILES["img2"]; imageupload($img1,$img2); Here is the actual function: function imageupload() { $images = func_get_args; for ($i = 0; $i < count($images); $i++) { $first = rand(100,1000000); $second = rand(0,10000000); $third = rand(0,1000); $file = $first . $second . "$third.jpg"; $fileerror = move_uploaded_file($_FILES["img1"]["tmp_name"], "../images/" . $file); if(!fileerror) { $error .= "<li>There was some problems with your file uploads, please try again. Should you experience any problems, contact the system adminstrator.</li>"; } } } Quote Link to comment Share on other sites More sharing options...
nvee Posted November 16, 2009 Author Share Posted November 16, 2009 Okay I have the problem ... The $_FILES["img1"] is references throughout the code $fileerror = move_uploaded_file($_FILES["img1"]["tmp_name"], "../images/" . $file); How will I be going about subtracting the name of the file from $_FILES so that it replaces the img1 in each loop? Quote Link to comment Share on other sites More sharing options...
PFMaBiSmAd Posted November 16, 2009 Share Posted November 16, 2009 func_get_args is a function and must be called correctly - $images = func_get_args(); You should be developing and debugging php code on a system with error_reporting set to E_ALL and display_errors set to ON in your master php.ini so that php will help you find errors in your code like the above. You will save a ton of time. Your code will be greatly simplified if you use an array - see example #3 at this link - http://us2.php.net/manual/en/features.file-upload.post-method.php Quote Link to comment Share on other sites More sharing options...
JustLikeIcarus Posted November 16, 2009 Share Posted November 16, 2009 The way i do it is just give the file inputs the name of img[]. Then $_FILES['img'] itself will be an array of the files. Quote Link to comment Share on other sites More sharing options...
mrMarcus Posted November 16, 2009 Share Posted November 16, 2009 we're going to simplify things here by using array's in your HTML form, then work with that array in your PHP script. for starters, change your HTML to this: <td><input type="file" name="img[]" id="img1" /></td> notice the [] .. this tells PHP that any incoming 'img' variables are an array. i also removed the 1 from the img1, 'cause it's not necessary anymore. do the same for the rest of your file input's: <td><input type="file" name="img[]" id="img1" /></td> <td><input type="file" name="img[]" id="img2" /></td> //etc... now, we must handle the incoming array/files with PHP: <?php while (list ($key, $value) = each ($_FILES['img']['name'])) { if (!empty ($value)) { #allowed file types - add/remove image types here; $allowed_types = array('image/gif', 'image/jpeg', 'image/pjpeg', 'image/png'); if (in_array ($_FILES['img']['type'][$key], $allowed_types)) { #upload directory; $upload_dir = '../images/'; #get file extension; $file_ext = pathinfo ($_FILES['img']['name'][$key]); $file_ext = strtolower ($file_ext['extension']); #create random name; $new_name = md5 (time()).'_'.rand (0,99999).'.'.$file_ext; #move file; if (!move_uploaded_file ($_FILES['img']['tmp_name'][$key], $upload_dir.$new_name)) { $errors .= '<li>There was some problems with your file uploads, please try again. Should you experience any problems, contact the system adminstrator.</li>'; } } else { $errors .= '<li>Incorrect file type. gif, jpeg, png only.</li>'; } } else { $errors .= '<li>Please upload an image.</li>'; } } ?> Quote Link to comment Share on other sites More sharing options...
nvee Posted November 16, 2009 Author Share Posted November 16, 2009 Thank you all for your response, it has helped a lot, and MrMarcus your post works 100&! Now my next questions is, how do I return a array back out of the function so that I can enter the filenames into the database seeing that there is new generated names. I assume again building an array and returning it out of the function? But how will I go about doing that? Quote Link to comment Share on other sites More sharing options...
mrMarcus Posted November 16, 2009 Share Posted November 16, 2009 you could change these lines: #create random name; $new_name = md5 (time()).'_'.rand (0,99999).'.'.$file_ext; #move file; if (!move_uploaded_file ($_FILES['img']['tmp_name'][$key], $upload_dir.$new_name)) { $errors .= '<li>There was some problems with your file uploads, please try again. Should you experience any problems, contact the system adminstrator.</li>'; } to: #create random name; $new_name[] = md5 (time()).'_'.rand (0,99999).'.'.$file_ext; #move file; if (!move_uploaded_file ($_FILES['img']['tmp_name'][$key], $upload_dir.$new_name[$key])) { $errors .= '<li>There was some problems with your file uploads, please try again. Should you experience any problems, contact the system adminstrator.</li>'; } now, $new_name will be an array populated with each image name. Quote Link to comment Share on other sites More sharing options...
nvee Posted November 16, 2009 Author Share Posted November 16, 2009 Thank you for your quick response MrMarcus! I am sorry, and I believe this will be my last question on this threat, but I get an error when I do the following (note that I made your code into a function) - I am obviously trying to return the array out of the function. When I do it, I get a Fatal Error, cannot read from [] Here is my array code: function imageupload() { while (list ($key, $value) = each ($_FILES['img']['name'])) { if (!empty ($value)) { #allowed file types - add/remove image types here; $allowed_types = array('image/gif', 'image/jpeg', 'image/pjpeg', 'image/png'); if (in_array ($_FILES['img']['type'][$key], $allowed_types)) { #upload directory; $upload_dir = '../images/'; #get file extension; $file_ext = pathinfo ($_FILES['img']['name'][$key]); $file_ext = strtolower ($file_ext['extension']); #create random name; $new_name[] = md5 (time()).'_'.rand (0,99999).'.'.$file_ext; #move file; if (!move_uploaded_file ($_FILES['img']['tmp_name'][$key], $upload_dir.$new_name[$key])) { $errors .= '<li>There was some problems with your file uploads, please try again. Should you experience any problems, contact the system adminstrator.</li>'; } } else { $errors .= '<li>Incorrect file type. gif, jpeg, png only.</li>'; } } else { $errors .= '<li>Please upload an image.</li>'; } } return $new_name[]; } Quote Link to comment Share on other sites More sharing options...
nvee Posted November 16, 2009 Author Share Posted November 16, 2009 ah, I guess I must not say "return $new_name[];" but "return $new_name"... I did this, and I did a var_dump on it, but it returns null ... Quote Link to comment Share on other sites More sharing options...
JustLikeIcarus Posted November 16, 2009 Share Posted November 16, 2009 I believe the new_name arr should be returned as simply return $new_name; and not return $new_name[]; Also you may want to declare $new_name in the main while loop because it appears that its scope is that of the inner if statement. Quote Link to comment Share on other sites More sharing options...
nvee Posted November 16, 2009 Author Share Posted November 16, 2009 I hate being such a newbie with PHP guys, I am really sorry! I still have such a looooong way too go! I added the $new_name = array(); just above the beginning of the while loop. I also changed the return to "return $new_name;" When I do an echo or a vardump on the $new_name, it returns null... I am really sorry for wasting your time like this, but do you have any suggestions on how to fix this? Quote Link to comment Share on other sites More sharing options...
mrMarcus Posted November 16, 2009 Share Posted November 16, 2009 just looking back at the code now, replace $new_name[] = md5... with $new_name[$key] = md5... that's not going to fix your code, but it completely ensures that the new array is being indexed as per the index of the original array. now, i have a feeling you are calling your function incorrectly. do this: $images = imageupload(); print_r ($images); $images is now your array of uploaded images/files. thing is, place the file upload within a function (as you have), your error messages will not be returned as you expect them to be. with that function, you are only returning the uploaded filenames. Quote Link to comment Share on other sites More sharing options...
JustLikeIcarus Posted November 16, 2009 Share Posted November 16, 2009 And for the record. I dont believe your wasting anyones time. We've all been there. Quote Link to comment Share on other sites More sharing options...
nvee Posted November 16, 2009 Author Share Posted November 16, 2009 Thank you for your kind comment JustLikeIcarus, i absolutely love the idea of coding something which works, its just getting to do it which sucks MrMarcus, youre a legend! It works like a charm. I see your point in the validation, so would it maybe be a good idea to break it down in 2 functions? I'm not sure if my logic is correct, but have a function structure like: funtion imagevalidation ( imageupload() if(!imageupload()) { echo "This will not work"; } else { echo "hooray!"; ) Fortunetaly this code will not be re-used, well not that I can think of atm. This is for a specific project, but for future image uploads I will rather create a seperate database for image_users and then make single file uploads, but then write the entries in the database to join the images to the users, which means I can "can" the array part completely and then just output the errors. Quote Link to comment Share on other sites More sharing options...
mrMarcus Posted November 16, 2009 Share Posted November 16, 2009 glad it's working. usually, the purpose of functions (in my opinion), is to be able to execute certain pieces of code over your entire site. they are especially handy when you pass arguments to them, making them multi-usage/multi-purpose. since you are only planning to use this one time on your site (as there are not always multiple needs for file/image uploads other than in a specific section of the site), you may want to consider just hard-coding the content of the function to the page, or sticking it in a separate file where you can include it. this way, your error handling will work fine as well. just doesn't seem necessary to put this into a function since you are not passing arguments and such. of course, this is just my opinion. my suggestion: 1. take the image upload code out of the function (lose the function altogether). 2. place that code (starting at the while()) in a separate file (call it file_upload.php), and then include it where you are currently calling the function: include ('/path/to/file_upload.php'); //where '/path/to/' is the path to that file 3. things would look like this now: <?php //your code leading up to file upload script; include ('/path/to/file_upload.php'); #we had an error; if (!empty ($errors)) { echo $errors; } else { echo 'Image successfully uploaded!'; } //continue with your code; ?> file_upload.php <?php while (list ($key, $value) = each ($_FILES['img']['name'])) { if (!empty ($value)) { #allowed file types - add/remove image types here; $allowed_types = array('image/gif', 'image/jpeg', 'image/pjpeg', 'image/png'); if (in_array ($_FILES['img']['type'][$key], $allowed_types)) { #upload directory; $upload_dir = '../images/'; #get file extension; $file_ext = pathinfo ($_FILES['img']['name'][$key]); $file_ext = strtolower ($file_ext['extension']); #create random name; $new_name[] = md5 (time()).'_'.rand (0,99999).'.'.$file_ext; #move file; if (!move_uploaded_file ($_FILES['img']['tmp_name'][$key], $upload_dir.$new_name[$key])) { $errors .= '<li>There was some problems with your file uploads, please try again. Should you experience any problems, contact the system adminstrator.</li>'; } } else { $errors .= '<li>Incorrect file type. gif, jpeg, png only.</li>'; } } else { $errors .= '<li>Please upload an image.</li>'; } } ?> now, there are other (arguably better) ways of handling errors than how you are currently doing it, but this should suffice for now. and this was just a suggestion .. if you want to do things within functions, go right ahead. the more practice with the functions the better. however, never make things harder than they need to be. functions are excellent for flexibility, but in a case like this, i don't see a function being necessary. Quote Link to comment Share on other sites More sharing options...
Recommended Posts
Join the conversation
You can post now and register later. If you have an account, sign in now to post with your account.