First draft of an image upload form

album

CREATE TABLE IF NOT EXISTS `album` (
  `album_id` int(255) NOT NULL AUTO_INCREMENT,
  `album_name` varchar(50) NOT NULL,
  `img_name` varchar(500) CHARACTER SET utf8 NOT NULL,
  `img_path` varchar(500) CHARACTER SET utf8 NOT NULL,
  `img_type` varchar(500) CHARACTER SET utf8 NOT NULL,
  `album_ident` varchar(500) NOT NULL,
  PRIMARY KEY (`album_id`)
) ENGINE=InnoDB DEFAULT CHARSET=latin1 AUTO_INCREMENT=1 ;

index.php

<head>
<script src="https://ajax.googleapis.com/ajax/libs/jquery/1.11.3/jquery.min.js"></script>
<link rel="stylesheet" href="https://ajax.googleapis.com/ajax/libs/jqueryui/1.11.4/themes/smoothness/jquery-ui.css">
<link rel="stylesheet" href="mystyle.css">
<script src="https://ajax.googleapis.com/ajax/libs/jqueryui/1.11.4/jquery-ui.min.js"></script>
</head>

<form  method="POST" action="upload.php" enctype="multipart/form-data" class="ajax" />
	<input type="file" name="file_img[]" multiple>
	<input type="text" name="folder">
<input type="submit" value="Create directory" id="makeit">
</form>
    <script>
      $('form.ajax').on('submit', function(){

	var that = $(this),
	url = that.attr('action'),
	type = that.attr('method'),
	data = {};
	that.find('[name]').each(function(index, value){
		var that = $(this),
			name = that.attr('name'),
			value = that.val();

		data[name] = value;	
	});
			 var fd= new FormData(this)
	$.ajax({
	url:"upload.php",
	type:"POST",
	processData:false,
	contentType: false,
	data:fd,
	success: function(response){
			console.log(response);
			 $('.myDiv').html(response);
	}
	});
	 return false; 
  });
</script>
  <body>
<div class="myDiv">
</div>
</body>

upload.php

<?php
error_reporting(E_ALL);

try{
	   $db = new PDO("mysql:host=localhost;dbname=mkdir", 'root', '');
		$db ->setAttribute(PDO::ATTR_ERRMODE, PDO::ERRMODE_EXCEPTION);
		}  catch(PDOException $e){
		echo $e->getMessage();
		die();				}

if (isset($_FILES['file_img'])) {
							$uploads = $_FILES["file_img"];
							$folderName = $_POST["folder"];
							$count = count($uploads['name']);

if (strlen($folderName) == ""  && !is_null($folderName)) 
			{		
				echo 'Album name is required';
			}
			else {			
		$length = 10;			
		$random = substr(str_shuffle("0123456789abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ"), 0, $length);
			echo $folderName . ' has been created <br /> <br />';					
            }
if ($uploads['error'][0] ===  UPLOAD_ERR_NO_FILE){
    $count = 0;
}	

if ($count) { // files 
    foreach($uploads['name'] as $key => $name) {
        if ($uploads['error'][$key] === UPLOAD_ERR_OK) {
            $size = $uploads["size"][$key];
            $type = $uploads["type"][$key];
            $tmp = $uploads["tmp_name"][$key];
            $name = $uploads["name"][$key];

			$temp_dir = "uploads/" . $name;	

            if ($type != "image/jpeg" && $type != "image/png" && $type != "image/gif") {
                echo $name . " Please upload jpg / png / gif <br />";
            } else if ($size > 3145728) {
                echo $name." is too large <br />";
            } else if (!empty($name)) {				
			 echo "Picture name ".$_FILES['file_img']['name'][$key]."<br />";	

			$sql = "INSERT INTO album (`album_name`, `img_name`, `img_path`, `img_type`, `album_ident`)
			VALUES (:folder, :filename, :filepath, :filetype, '$random');
	";

		$query = $db->prepare($sql);
		$query->execute(array(
			':folder'=> $folderName,
			':filename'=> $name,
			':filepath'=> $temp_dir,
			':filetype'=> $type
		));		

		move_uploaded_file($tmp, "$temp_dir");
			}	
        }		
    }
}else{	
	echo "<p>No files submitted</p>";
}
}



?>

Still relatively new to all of this, it works fine but I'm sure it could be improved upon.

    I could nitpick a few stylistic things, but my main suggestion would be to move the insert query prepare to before the start of the foreach loop, as it only needs to be done once.

    Also, it would be cleaner to have the try{} apply to all the PHP code in the handler, and that way you can catch{} any exceptions thrown by PDO (or anything else) and handle them as desired.

    From a big-picture viewpoint, if this is something you might want to re-use in multiple applications, it might be worth considering how to package things up into one or more classes so that you could get all OOP about it (at least the actual upload-handler part).

    A couple of those nit-picks:

    • Assigning a value from a super-global array element (e.g. from $FILES or $POST) to a simple scalar variable doesn't gain you anything while adding a tiny bit of extra processing and memory usage. I'd only recommend doing it if you are also doing some sort of filtering/altering of the input values; otherwise just use the actual source array variable/key where you actually need its value.

    • A closing "?>" at the very end of a PHP file is redundant, and it's a good habit to get out of, as sometimes it causes problems (mainly if you, your editor, or FTP client sticks a newline after it).

      Thanks, working on those changes now, yes the ?> at the end of the code is a habit I got into awhile ago and keep forgetting to stop doing every so often (as noted)

      My try was based on:
      Example two, (further reading required I suspect)
      http://php.net/manual/en/pdo.connections.php

      I will attempt to convert to classes tonight, still not completely comfortable with that idea but I guess now is as good a time to learn as any.

      My next step after that was completed was to allow for thumbnails to be displayed on change and images removed, I thought that might be the direction to go on.

      Over all thanks for the input I'll work on improving this tonight

        I feel like you need to pick an indentation pattern and stick with it. You have braces on the line with their control structure and on the next line. You have code indented, not indented and de-indented from the control structure it belongs to.

            if (strlen($folderName) == ""  && !is_null($folderName))  

        This makes no sense, strlen returns a number but you check if its equal to an empty string.

                    $query->execute(array( 
                        ':folder'=> $folderName, 
                        ':filename'=> $name, 
                        ':filepath'=> $temp_dir, 
                        ':filetype'=> $type 
                    ));         
        
                move_uploaded_file($tmp, "$temp_dir"); 

        Both of these statements may fail independently, I would think about making sure neither the file or db entry exist if either fail. (IE if inserting fails, delete the file... if moving the file fails, delete the row inserted). "$variable" is redundant and requires string interpolation, when you can just write $variable instead.

        It kind of hurts my head to look at this closely due to the inconsistent formatting, I would definitely fix that first and repost as we might have more things. Also, please don't take it as mean! I've gotten some scathing reviews from people myself, and its all about learning so you don't get the same reviews next time 🙂

          Derokorian;11048227 wrote:

          I feel like you need to pick an indentation pattern and stick with it. You have braces on the line with their control structure and on the next line. You have code indented, not indented and de-indented from the control structure it belongs to....

          I copy/pasted it into PHPStorm, hit ctrl-alt-L, et voila. 🙂

          <?php
          error_reporting(E_ALL);
          
          try {
              $db = new PDO("mysql:host=localhost;dbname=mkdir", 'root', '');
              $db->setAttribute(PDO::ATTR_ERRMODE, PDO::ERRMODE_EXCEPTION);
          } catch (PDOException $e) {
              echo $e->getMessage();
              die();
          }
          
          if (isset($_FILES['file_img'])) {
              $uploads = $_FILES["file_img"];
              $folderName = $_POST["folder"];
              $count = count($uploads['name']);
          
          if (strlen($folderName) == "" && !is_null($folderName)) {
              echo 'Album name is required';
          } else {
              $length = 10;
              $random = substr(str_shuffle("0123456789abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ"), 0, $length);
              echo $folderName . ' has been created <br /> <br />';
          }
          if ($uploads['error'][0] === UPLOAD_ERR_NO_FILE) {
              $count = 0;
          }
          
          if ($count) { // files 
              foreach ($uploads['name'] as $key => $name) {
                  if ($uploads['error'][$key] === UPLOAD_ERR_OK) {
                      $size = $uploads["size"][$key];
                      $type = $uploads["type"][$key];
                      $tmp = $uploads["tmp_name"][$key];
                      $name = $uploads["name"][$key];
          
                      $temp_dir = "uploads/" . $name;
          
                      if ($type != "image/jpeg" && $type != "image/png" && $type != "image/gif") {
                          echo $name . " Please upload jpg / png / gif <br />";
                      } else if ($size > 3145728) {
                          echo $name . " is too large <br />";
                      } else if (!empty($name)) {
                          echo "Picture name " . $_FILES['file_img']['name'][$key] . "<br />";
          
                          $sql = "INSERT INTO album (`album_name`, `img_name`, `img_path`, `img_type`, `album_ident`)
                      VALUES (:folder, :filename, :filepath, :filetype, '$random');
              ";
          
                          $query = $db->prepare($sql);
                          $query->execute(array(
                              ':folder' => $folderName,
                              ':filename' => $name,
                              ':filepath' => $temp_dir,
                              ':filetype' => $type
                          ));
          
                          move_uploaded_file($tmp, "$temp_dir");
                      }
                  }
              }
          } else {
              echo "<p>No files submitted</p>";
          }
          }
          

            For the fun(?) of it, I decided to play around with some sort of OOP approach. When you want to see how my (warped) mind approaches this, take a look at https://github.com/nogdog/Image-Upload , noting that it's a work in progress which may change completely as I think about it more, or may die at some point from lack of interest. 🙂

              I have been working on it some more (learnt a few new tricks not all included here)

              <?php
              error_reporting(E_ALL);
              
              try {
                  $db = new PDO("mysql:host=localhost;dbname=mkdir", 'root', '');
                  $db->setAttribute(PDO::ATTR_ERRMODE, PDO::ERRMODE_EXCEPTION);
              
              if (isset($_FILES['file_img'])) {
                  $uploads = $_FILES["file_img"];
                  $folderName = $_POST["folder"];
                  $count = count($uploads['name']);
              
              $userInput = trim($folderName);
              $userInput = htmlspecialchars($userInput, ENT_COMPAT,'ISO-8859-1', true);
              
                  if ($uploads['error'][0] === UPLOAD_ERR_NO_FILE) {
                      $count = 0;
                      echo "<p>No files submitted</p>";
                  }
              	if (ctype_space($folderName)) {
                  echo "Album name is required <br />";
              }
              	else if (empty($folderName)) {
                      echo 'Album name is required <br />';
                  } else {
                      if ($count) { // files
                          $length = 10;
                          $random = substr(str_shuffle("0123456789abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ"), 0, $length);
                          $sql = "INSERT INTO album (`album_name`, `img_name`, `img_path`, `img_type`, `album_ident`)
                          VALUES (:folder, :filename, :filepath, :filetype, '$random');
              			";
                          foreach ($uploads['name'] as $key => $name) {
                              if ($uploads['error'][$key] === UPLOAD_ERR_OK) {
                                  $size = $uploads["size"][$key];
                                  $type = $uploads["type"][$key];
                                  $tmp = $uploads["tmp_name"][$key];
                                  $name = $uploads["name"][$key];
                                  $temp_dir = "uploads/images/" . $name;
              
                                  $dir = 'uploads/images';
              
                                  if (!is_file($dir) && !is_dir($dir)) {
              						mkdir($dir, 0777, true);
              					}	
                                      if ($type != "image/jpeg" && $type != "image/png" && $type != "image/gif") {
                                          echo $name . " Please upload jpg / png / gif <br />";
                                      } else if ($size > 3145728) {
                                          echo $name . " is too large <br />";
                                      } else if (!empty($name)) {
                                          echo "Picture name " . $_FILES['file_img']['name'][$key] . "<br />";
                                          $query = $db->prepare($sql);
                                          $query->execute(array(
                                              ':folder' => $folderName,
                                              ':filename' => $name,
                                              ':filepath' => $temp_dir,
                                              ':filetype' => $type
                                          ));
                                          move_uploaded_file
                                          (
                                              $tmp,
                                              "$temp_dir"
                                          );
                                      }
              
                                  }
                              }
                          }
                      }
                  }
              }catch (PDOException $e) {
              echo $e->getMessage();
              die();
              }
              

              Also nit-picking is good I like nit-picking if it helps me improve 🙂

              Still need to make it oop-ish..

              Also I had never heard of PHPstorm had been using notepad++, it seems quite good.

                Editors and IDEs are all a matter of personal taste to some degree. If you get 10 developers together in a room, there will likely be at least 8 different "best" editors. 🙂 I just started using PHPStorm in the last couple months or three, and really like it a lot. It may be overkill for a small project with only a few files, and it's not free -- though you can keep downloading/installing the latest nightly build every 4 weeks or so.

                  I have used the free version of PHP Designer before.
                  But for now I use only only Notepad++ for everything. It has code highlighting with colors for PHP.
                  So, I do like cluelessPHP does.

                    NogDog;11048231 wrote:

                    I copy/pasted it into PHPStorm, hit ctrl-alt-L, et voila. 🙂

                    Really loving this feature of PhpStorm.

                    I recently had to make changes to a website of a former coworker who we let go a couple months ago. He is one of those people that write their CSS rules all in one line. So frustrating. Instead of causing myself headaches I was able to reformat the code with a couple keystrokes.

                      PHPStorm does seem nice, before I move onto making more oop-ish, going to try and fix the front end to be more interactive (unless someone is kind enough to hit me with a bus)

                        Just a personal note, but I wouldn't insert the record in the DB until after the file is moved. There's a much higher chance that the moving of the file is going to fail than the insert into the database, at which point you've got an orphan record hanging out, mucking things up. Even if the insert does fail, it's better to have an orphan file that the system doesn't really know is there than an orphan record the system is trying to use, you know?

                          cluelessPHP;11048407 wrote:

                          Sorry in advance for the ads, it's a free server.

                          That's what AdBlock Plus is for. 😉

                            23 days later

                            I haven't forgotten about this but have some course work to finish first!

                              2 years later

                              Geez...I don't even remember doing that. 🙂

                                Bonesnap;11048259 wrote:

                                He is one of those people that write their CSS rules all in one line.

                                Were I Catholic, I'd petition the Pope for a new, deeper level of hell for such a person. :eek:

                                  Write a Reply...