# Movie database browser

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']))
{
}

$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))
{
}

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?

-

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.

-
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 :)) – Aleksandar Jan 5 '13 at 11:05
Doesnt matter really :) This can happen in any language. It just happened to be written in php when I came across this problem. – user1534664 Jan 5 '13 at 11:53
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? – user1534664 Jan 5 '13 at 11:59
"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. – s_ha_dum Jan 5 '13 at 15:56
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. – mseancole Jan 7 '13 at 17:35