Cleaner Code

PHP programming forum. Ask questions or help people concerning PHP code. Don't understand a function? Need help implementing a class? Don't understand a class? Here is where to ask. Remember to do your homework!

Moderator: General Moderators

Post Reply
icesolid
Forum Regular
Posts: 502
Joined: Mon May 06, 2002 9:36 pm
Location: Buffalo, NY

Cleaner Code

Post by icesolid »

I am currently running a script now that I wrote for uploading more than one image file at once.

The user MUST select at least one image file to upload.

The maximum number of files that are allowed is FOUR. I have four file inputs and a submit button to send the data.

So to the question...My script works and works just fine, however I was just wondering how some of you users on this forum would code that script.

My current code is pretty lenghy and involves a lot of (if else if else if else if else) crap! I guess what I am just trying to find a cleaner way of coding this.
User avatar
hawleyjr
BeerMod
Posts: 2170
Joined: Tue Jan 13, 2004 4:58 pm
Location: Jax FL & Spokane WA USA

Post by hawleyjr »

Please post your code so we can look through it...
User avatar
Chris Corbyn
Breakbeat Nuttzer
Posts: 13098
Joined: Wed Mar 24, 2004 7:57 am
Location: Melbourne, Australia

Post by Chris Corbyn »

Very basic and untested but....

Code: Select all

<input type="file" name="foo[]" />
<input type="file" name="foo[]" />
<input type="file" name="foo[]" />
<input type="file" name="foo[]" />

Code: Select all

if (!empty($_FILES['foo']) && is_array($_FILES['foo']))
{
    foreach ($_FILES['foo'] as $file)
    {
        $filename = time().$file['name'];
        if (move_uploaded_file($file['tmp_name'], '/home/username/public_html/img/'.$filename))
        {
            $query = "insert into tbl_name (userid, image_path) values ('{$_SESSION['user']}', '{$filename}')";
            mysql_query($query) or die(mysql_error());
        }
        else header('Location: filename.php?error');
    }
}
else echo 'You have to upload some images to use this page';
icesolid
Forum Regular
Posts: 502
Joined: Mon May 06, 2002 9:36 pm
Location: Buffalo, NY

My Code

Post by icesolid »

AS YOU CAN SEE I AM TRYING FOR A EASIER WAY TO ACCOMPLISH THINGS, MY CODE SUCKS!!! :x

Code: Select all

<?php
if($_POST["cmd"] == "add") {
    /******************************
    ********  ADD COMMAND  ********
    *******************************/

        $uploadfile1 = "/home/jwcarney/public_html/$directoryname/" . basename($_FILES["userfile1"]["name"]);
        $uploadfile2 = "/home/jwcarney/public_html/$directoryname/" . basename($_FILES["userfile2"]["name"]);
        $uploadfile3 = "/home/jwcarney/public_html/$directoryname/" . basename($_FILES["userfile3"]["name"]);
        $uploadfile4 = "/home/jwcarney/public_html/$directoryname/" . basename($_FILES["userfile4"]["name"]);

        if(file_exists($uploadfile2) || !exif_imagetype($_FILES["userfile2"]["tmp_name"]) == "2" || !exif_imagetype($_FILES["userfile2"]["tmp_name"]) == "1") {
            $file2 = false;
        }

        if(file_exists($uploadfile3) || !exif_imagetype($_FILES["userfile3"]["tmp_name"]) == "2" || !exif_imagetype($_FILES["userfile3"]["tmp_name"]) == "1") {
            $file3 = false;
        }

        if(file_exists($uploadfile4) || !exif_imagetype($_FILES["userfile4"]["tmp_name"]) == "2" || !exif_imagetype($_FILES["userfile4"]["tmp_name"]) == "1") {
            $file4 = false;
        }
    
        $sortorder = strtoupper($_POST["sortorder"]);
        $result = mysql_query("SELECT sortorder FROM $tablename WHERE sortorder='$sortorder'");
        $row = mysql_fetch_array($result);

        if($row == true) {
            $status = "<b><u>ERROR:</u></b> The sort order <i>$sortorder</i> already exists.\n";
        } elseif(!$_POST["sortorder"]) {
            $status = "<b><u>ERROR:</u></b> You must fill out the <i>sort order</i> field.\n";    
        } elseif(!$_POST["company"]) {
            $status = "<b><u>ERROR:</u></b> You must enter a valid <i>company</i> address.\n";
        } elseif(!$_POST["item"]) {
            $status = "<b><u>ERROR:</u></b> You must fill out the <i>item</i> field.\n";
        } elseif(!$_POST["model"]) {
            $status = "<b><u>ERROR:</u></b> You must fill out the <i>model</i> field.\n";
        } elseif(!$_POST["price"]) {
            $status = "<b><u>ERROR:</u></b> You must fill out the <i>price</i> field.\n";
        } elseif(!$_POST["description"]) {
            $status = "<b><u>ERROR:</u></b> You must fill out the <i>description</i> field.\n";
        } elseif(file_exists($uploadfile1) || !exif_imagetype($_FILES["userfile1"]["tmp_name"]) == "2" || !exif_imagetype($_FILES["userfile1"]["tmp_name"]) == "1") {
            $status = "<b><u>ERROR:</u></b> The file name <i>" . $_FILES["userfile1"]["name"] . "</i> already exists or is not an <i>.JPG</i> or <i>.GIF</i> image file.\n";
        } elseif($file2 = false && !$_FILES["userfile2"]["name"] == "") {
            $status = "<b><u>ERROR:</u></b> The file name <i>" . $_FILES["userfile2"]["name"] . "</i> already exists or is not an <i>.JPG</i> or <i>.GIF</i> image file.\n";
        } elseif($file3 == false && !$_FILES["userfile3"]["name"] == "") {
            $status = "<b><u>ERROR:</u></b> The file name <i>" . $_FILES["userfile3"]["name"] . "</i> already exists or is not an <i>.JPG</i> or <i>.GIF</i> image file.\n";
        } elseif($file4 == false && !$_FILES["userfile4"]["name"] == "") {
            $status = "<b><u>ERROR:</u></b> The file name <i>" . $_FILES["userfile4"]["name"] . "</i> already exists or is not an <i>.JPG</i> or <i>.GIF</i> image file.\n";
        } else {
            $sortorder = strtoupper($_POST["sortorder"]);
            
            if($file2 = true) {    
                move_uploaded_file($_FILES["userfile2"]["tmp_name"], $uploadfile2);
            }

            if($file3 = true) {    
                move_uploaded_file($_FILES["userfile3"]["tmp_name"], $uploadfile3);
            }

            if($file4 = true) {    
                move_uploaded_file($_FILES["userfile4"]["tmp_name"], $uploadfile4);
            }

            move_uploaded_file($_FILES["userfile1"]["tmp_name"], $uploadfile1);
            
            mysql_query("INSERT INTO $tablename (sortorder,file1,file2,file3,file4,company,item,model,price,description) VALUES ('$sortorder','" . $_FILES["userfile1"]["name"] . "','" . $_FILES["userfile2"]["name"] . "','" . $_FILES["userfile3"]["name"] . "','" . $_FILES["userfile4"]["name"] . "','" . $_POST["company"] . "','" . $_POST["item"] . "','" . $_POST["model"] . "','" . $_POST["price"] . "','" . $_POST["description"] . "')");
            mysql_query("CHECK TABLE $tablename");
            mysql_query("ANALYZE TABLE $tablename");
            mysql_query("REPAIR TABLE $tablename");
            mysql_query("OPTIMIZE TABLE $tablename");

            $status = "<b><u>MESSAGE:</u></b> The vehicle has been added.\n";
        }

        echo $status;
        echo "<br><br>\n";
        echo "Please <a href=\"" . $_SERVER["PHP_SELF"] . "\">click here</a> to go back.\n";
    }
?>
Last edited by icesolid on Mon Dec 05, 2005 5:20 pm, edited 1 time in total.
User avatar
Chris Corbyn
Breakbeat Nuttzer
Posts: 13098
Joined: Wed Mar 24, 2004 7:57 am
Location: Melbourne, Australia

Post by Chris Corbyn »

Yeah you've got sanity checking in yours (which is excellent). Pretty much the only thing I can suggest is using a loop like I've done so you're not repeating code and name your file fields in the form as arrays like so name="foo[]" ;)

EDIT | And I forgot to basename() the filename :P Heh... too much rushing... too close to bed time :D
icesolid
Forum Regular
Posts: 502
Joined: Mon May 06, 2002 9:36 pm
Location: Buffalo, NY

Thank You

Post by icesolid »

Yeah I guess I could use a loop, however I've tried before but I just <span style='color:blue' title='I&#39;m naughty, are you naughty?'>smurf</span> it up. That's why I was asking for an example.

I'm not that great at PHP, I just learn and do as I need to kind of thing.
icesolid
Forum Regular
Posts: 502
Joined: Mon May 06, 2002 9:36 pm
Location: Buffalo, NY

Hmhmhmmh....

Post by icesolid »

Still no loop examples for my situation...Please?
d3ad1ysp0rk
Forum Donator
Posts: 1661
Joined: Mon Oct 20, 2003 8:31 pm
Location: Maine, USA

Post by d3ad1ysp0rk »

Check d11wtq's first post.
User avatar
Chris Corbyn
Breakbeat Nuttzer
Posts: 13098
Joined: Wed Mar 24, 2004 7:57 am
Location: Melbourne, Australia

Post by Chris Corbyn »

Code: Select all

if (if($_POST["cmd"] == "add")
{
	$sortorder = strtoupper($_POST["sortorder"]);
    $result = mysql_query("SELECT sortorder FROM $tablename WHERE sortorder='$sortorder'");
    $row = mysql_fetch_array($result);
		
	if (!empty($_FILES['userfile']) && is_array($userfile))
	{
		foreach ($_FILES['userfile']  as $i => $file_array)
		{
			$uploadfile = "/home/jwcarney/public_html/$directoryname/" . basename($file_array["name"]);
			
			if(file_exists($uploadfile) || !exif_imagetype($file_array["tmp_name"]) == "2" || !exif_imagetype($file_array["tmp_name"]) == "1")
			{
				move_uploaded_file($file_array["tmp_name"], $uploadfile);
				
				if (!empty($_POST["sortorder"])
				&& !empty($_POST["company"])
				&& !empty($_POST["item"])
				&& !empty($_POST["model"])
				&& !empty($_POST["price"])
				&& !empty($_POST["price"]))
				//Start if
				mysql_query("
					INSERT INTO
					$tablename (
						sortorder,
						file{$i},
						company,
						item,
						model,
						price,
						description)
					VALUES (
						'$sortorder',
						'" . $file_array["name"] . "',
						'" . $_POST["company"] . "',
						'" . $_POST["item"] . "',
						'" . $_POST["model"] . "',
						'" . $_POST["price"] . "',
						'" . $_POST["description"] . "')") or die(mysql_error());
				//End if
				
        	}
		}
		mysql_query("CHECK TABLE $tablename");
		mysql_query("ANALYZE TABLE $tablename");
		mysql_query("REPAIR TABLE $tablename");
		mysql_query("OPTIMIZE TABLE $tablename");
		echo $status;
        echo "<br><br>\n";
        echo "Please <a href=\"" . $_SERVER["PHP_SELF"] . "\">click here</a> to go back.\n";
	}
}
I havent gone through all your sanity checking (I'm off to bed) but hopefully you get the gist of it :)
Post Reply