File uploader in php

Question

I can't get a peer review where I work, I was wondering how my code was and how could it be improved.

https://github.com/lamondea/uploader

Is it a good practice to use functions in classes or should it be replaced by external function that use the object returned by the class:

class _export
{ 
 public $nbr;
 public $file;
 public function __construct($file,$nbr=0){
 if($nbr != 0 && ($file->ext == 'jpg' || $file->ext == 'png' || $file->ext == 'gif'))
 thumbnailer($file,$nbr);
 else {
 move_uploaded_file($file->tmp_name, ($file->loc).($file->name));
 return $file->name;
 }
 }
}

• Lamonde: I really never used peer review and I just started using classes, so I guess the whole /class/uploader.class.php would be the important for that matter. I will add a part, but still it would be hard to choose a part since the uploader work.
• Kinjal: Two general things about the code, 1) class name shouldn't start with "_", 2) Looks like you are doing the validation in constructor which is not good. Let the constructor just for initialization stuff.
• Lamonde: Is there a norm about naming the classes?

Answer 1

I'm not sure if using classes is better or worse but I am starting to see files being called as functions more than a single file with function within. This is a more common upload script.

$allowedExts = array("gif", "jpeg", "jpg", "png");
$temp = explode(".", $_FILES["file"]["name"]);
$extension = end($temp);
if ((($_FILES["file"]["type"] == "image/gif")
 || ($_FILES["file"]["type"] == "image/jpeg")
 || ($_FILES["file"]["type"] == "image/jpg")
 || ($_FILES["file"]["type"] == "image/pjpeg")
 || ($_FILES["file"]["type"] == "image/x-png")
 || ($_FILES["file"]["type"] == "image/png"))
 && ($_FILES["file"]["size"] < 20000)
 && in_array($extension, $allowedExts)){
 if ($_FILES["file"]["error"] > 0){
 echo "Error: " . $_FILES["file"]["error"] . "<br>";
 }
 else{
 echo "Upload: " . $_FILES["file"]["name"] . "<br>";
 echo "Type: " . $_FILES["file"]["type"] . "<br>";
 echo "Size: " . ($_FILES["file"]["size"] / 1024) . " kB<br>";
 echo "Stored in: " . $_FILES["file"]["tmp_name"];
 }
}
else{
 echo "Invalid file";
}

• palacsint: The $allowedExts array is a good idea! You could use a similar array for $_FILES["file"]["type"] too.
• epool: Agreed, i would shorten this and say if($_FILES["file"]["type"] is in $ArrayofMetaData) or something to that affect. It would look much cleaner.