Movie database browser

Question

My code looks like "spaghetti code". I have yet to use OOP in PHP but I am really interested in how a good programmer would handle this many if statements. It looks like a mess.

<?php
 session_start();
 include './functions.php';
 if(empty($_FILES['browseMovie']) || empty($_FILES['browseThumbnail']) || empty($_GET['title']) || empty($_GET['description']))
 {
 die('Please insert all fields.');
 }
 $filePartsMovie = pathinfo($_FILES['browseMovie']['name']);
 $filePartsThumbnail = pathinfo($_FILES['browseThumbnail']['name']);
 if($filePartsMovie['extension'] !== "mp4")
 {
 die('Video has to be a .mp4 file.');
 }
 if($filePartsThumbnail['extension'] !== "jpg" && $filePartsThumbnail['extension'] !== "png")
 {
 die('Thumbnail has to be in jpg/jpeg or png format.');
 }
 if($_FILES['browseMovie']['size'] + $_FILES['browseThumbnail']['size'] > 1600600000)
 {
 die('filesize of video and thumbnail = higher 1.5 GB.');
 }
 $id = $_SESSION["id"];
 $name = $_SESSION["username"];
 $folderName = generateUniqueFolder();
 $title = mysql_real_escape_string($_GET['title']);
 $description = mysql_real_escape_string($_GET['description']);
 // the original file name will be stored in the database as well
 $thumbFileName = basename($_FILES['browseThumbnail']['name']);
 $vidFileName = basename($_FILES['browseMovie']['name']);
 $userDir = "../users/$name/";
 $videoDir = $userDir . "videos/" . $folderName . "/";
 $uploadMovieDir = $videoDir . $vidFileName;
 $uploadThumbnailDir = $videoDir . $thumbFileName;
 if(!isLoggedIn() || !is_dir($userDir))
 {
 die('User does not have own folder. Please contact the administrator.');
 }
 if(!mkdir($videoDir))
 {
 die('Could not create unique video folder.');
 }
 if(!move_uploaded_file($_FILES['browseMovie']['tmp_name'], $uploadMovieDir))
 {
 rmdir($videoDir);
 die('Could not upload the video correctly.');
 }
 if(!move_uploaded_file($_FILES['browseThumbnail']['tmp_name'], $uploadThumbnailDir))
 {
 removeTreeDir($videoDir);
 die('Could not upload the thumbnail correctly.');
 }
 $con = connectDb();
 $videoQuery = "INSERT INTO video(userId, title, description, foldername, vidFileName, thumbFileName) VALUES ($id,'$title','$description','$folderName', '$vidFileName', '$thumbFileName')";
 if(!mysql_query($videoQuery))
 {
 removeTreeDir($videoDir);
 die('Could not add the video to the database.');
 }
 mysql_close($con);
 echo "true";
?>

Somewhere in the middle of my code I also need a bunch of variables. How would one clean that up? Create an object and aggregate them together?

Answer 1

Your first if can be simplifed with any function.

function any ($list, $pred) 
{
 foreach ($list as $value) 
 {
 if (call_user_func ($pred, $value) == TRUE)
 return TRUE;
 }
 return FALSE;
}

So it will be :

 $toTest = [$_FILES['browseMovie'], 
 $_FILES['browseThumbnail'], 
 $_GET['title'], 
 $_GET['description'] ]
 if (any ($toTest, "empty")) die ("Please insert all fields")

Then you can simplify the code even more by creating a validate type function

function validateFile ($filePartsMovie, $filePartsThumbnail) 
{
 if($filePartsMovie['extension'] !== "mp4")
 {
 die('Video has to be a .mp4 file.');
 }
 if($filePartsThumbnail['extension'] !== "jpg" &&
 $filePartsThumbnail['extension'] !== "png")
 {
 die('Thumbnail has to be in jpg/jpeg or png format.');
 } 
 if($_FILES['browseMovie']['size'] + $_FILES['browseThumbnail']['size'] > 1600600000)
 {
 die('filesize of video and thumbnail = higher 1.5 GB.');
 }
}

Then you can create a validate function for the rest of the code.

TLDR is to split your code in more separate functions.

BTW I think you do not need brackets when having only one statement after if.

• Aleksandar: I do not know PHP so this is from my general programming knowledge. It might not work, but the idea behind the code I posted is good (I hope :))
• user1534664: Doesnt matter really :) This can happen in any language. It just happened to be written in php when I came across this problem.
• user1534664: BTW, I like your validateFile() function. That would definitely make it more organized. :) I dont know about the top one, though. I only need to check if they're empty once. Is that worth the effort adding extra code?
• s_ha_dum: "BTW I think you do not need brackets when having only one statement after if." -- You don't, but I nearly always use them for readability.
• mseancole: Indeed, braceless syntax is allowed, but is usually frowned upon. Additionally that short array syntax ([] instead of array()) is relatively new to PHP (5.4) and may be confusing if someone is unfamiliar with it and attempts to use it with an older version.