\$\begingroup\$
\$\endgroup\$
5
I'm working on an upload file program in PHP in OOP style. I need some feedback about code.
index.php
<?php
require_once('./lib/upload.php');
?>
<?php
if(isset($_FILES['file'])){
$fileupload = new upload();
if(!$fileupload -> sizeck()){
if($fileupload -> extens()){
if($fileupload -> uploadfile()){
echo 'Fisierul a fost uploadat';
}
}
}
}
?>
<html>
<head></head>
<body>
<form align="center" enctype="multipart/form-data" action="<?php echo htmlspecialchars($_SERVER["PHP_SELF"]); ?>" method="post">
Select upload file: <input type="file" name="file" required="yes" />
<input type="submit" value="Trimite" />
<p>
<p>
<p>
<br>If you want to view file and download <a href="./upload/"> click here </a>
</form>
</body>
</html>
lib/upload.php
<?php
class upload{
public $src = "upload/";
public $tmp;
public $filename;
public $typefl;
public $uploadfile;
public $type = array("php", "css", "js", "html", "htm", ".php");
function __construct(){
$this -> filename = $_FILES["file"]["name"];
$this -> tmp = $_FILES["file"]["tmp_name"];
$this -> uploadfile = $this -> src . basename($this -> filename);
}
public function sizeck(){
if($_FILES["file"]["size"] > 50000000){
echo "Fisier prea mare";
return true;
}
}
public function extens(){
$this -> typefl = pathinfo($this -> filename, PATHINFO_EXTENSION);
if(in_array($this -> typefl, $this -> type)){
echo "Fisier nepermis!!!";
return false;
}
else{
return true;
}
}
public function uploadfile(){
if(move_uploaded_file($this -> tmp, $this -> uploadfile)){
return true;
}
}
}
?>
Jamal
35.2k13 gold badges134 silver badges238 bronze badges
asked Jul 16, 2015 at 10:56
1 Answer 1
\$\begingroup\$
\$\endgroup\$
Your code is not really object oriented, but here are some improvements:
- Use upper case names for classes (see CamelCase)
- Do not use an empty spaces between varibale
$objectand the method pointer ->, use e. g.$object->method() - Do not use global variables like
$_FILESin your classes. You should add a parameter which contains the file. - Use human readable method names, like e. g.
verifyExtension()instead ofextens() - Make your target uploading path on your server configurable (reusability)
- Do not use too many nested
ifconditions. - Simpilfy your
ifnesting, e. g. combine your conditions and make configurable - You can seperate your Validation and Upload, in two classes e. g.
FileValidationandFileUpload - You do not use your type checking attribute
$type(type checking is missing) - Do not use
echoin your file uploading and validation classes.
These are only the most important improvements.
Quill
12k5 gold badges41 silver badges93 bronze badges
lang-php
.phpfiles? Because that seems very dangerous. You might want to include some kind of MIUME checker. There are a couple ones out there using CURL to get the actual MIME type, but uploads are always dangerous. \$\endgroup\$