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

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;
}
}
}

-
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. –  Lamonde Feb 26 '13 at 17:53
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. –  Kinjal Feb 27 '13 at 8:36
Is there a norm about naming the classes? –  Lamonde Feb 27 '13 at 13:46

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";
}

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