Advice on Handling No Photo

[doubledee]

I have this modest Function that is supposed to handle when a User hasn't uploaded a Photo, or has, but it isn't yet approved...

 

function validatePhoto($photoName, $photoApproved){
	// Check for Photo.
	if (is_null($photoName)){
		// Photo Not Found.
		$safeImage = "NoImageAvailable_100x76.png";

	}else{
		// Photo Found.
		// Check if Approved.
		if ($photoApproved==1){
			// Photo Approved.
			$safeImage = $photoName;
		}else{
			// Photo Pending.
			$safeImage = "PhotoPendingApproval_100x76.png";
		}
	}

	return $safeImage;
}//End of validatePhoto

 

 

It seems my use of is_null() maybe isn't the best choice.

 

For example, I just deleted a Photo out of the table using phpMyAdmin, and it left an Empty String in the spot, messing up the output I expected by instead showing the HTML ALT attribute instead of "No Image Available."

 

Any suggestions on how to tighten up this Function?

 

Thanks,

 

 

Debbie

 

[trq]

Your function will fail if nothing is passed in as the first argument so yeah, there is not likely any need to check to see if it's null.

[doubledee]

Your function will fail if nothing is passed in as the first argument so yeah, there is not likely any need to check to see if it's null.

 

So what do I do then??

 

If someone never uploaded a Photo, "photo_name" will be NULL...

 

 

Debbie

 

[doubledee]

How about this...

function validatePhoto($photoName, $photoApproved){
	// Check for Photo.
	if (is_null($photoName) || (empty($photoName))){
		// Photo Not Found.
		$safeImage = "NoImageAvailable_100x76.png";

	}else{
		// Photo Found.
		// Check if Approved.
		if ($photoApproved==1){
			// Photo Approved.
			$safeImage = $photoName;
		}else{
			// Photo Pending.
			$safeImage = "PhotoPendingApproval_100x76.png";
		}
	}

	return $safeImage;
}//End of validatePhoto

 

 

Debbie

 

[The Little Guy]

all of the following are considered empty:

 

"" (an empty string)

0 (0 as an integer)

0.0 (0 as a float)

"0" (0 as a string)

NULL

FALSE

array() (an empty array)

var $var; (a variable declared, but without a value in a class)

 

So really you don't even need the is_null()

 

might also want to consider removing all spaces before passing to empty()

 

empty: ""

not empty: " " or "      "

 

Edit:

 

I am not sure if you already have the file saved, but another function to use would be file_exists if you have it saved

[doubledee]

all of the following are considered empty:

 

"" (an empty string)

0 (0 as an integer)

0.0 (0 as a float)

"0" (0 as a string)

NULL

FALSE

array() (an empty array)

var $var; (a variable declared, but without a value in a class)

 

So really you don't even need the is_null()

 

Okay.

 

 

might also want to consider removing all spaces before passing to empty()

 

empty: ""

not empty: " " or "      "

 

You lost me.

 

 

Edit:

 

I am not sure if you already have the file saved, but another function to use would be file_exists if you have it saved

 

Interesting point.

 

My Upload Script saves the file to my "uploads" directory, and then stores the filename in the "photo_name" field in my database.

 

So I figured I should check the database, since if I have a value there, then the File *should* exist, and I need the value in the database.

 

Won't changing my code to just use empty() accomplish what I want and need?

 

 

Debbie

 

[The Little Guy]

you could change it to empy(), but that doesn't mean that the string passed to it is a real existing file.

 

*should* and *do* are two different things, any number of errors could have gone wrong while saving the file, and the file didn't get saved, or maybe someone deleted the file without you knowing.

 

maybe your table cell only stores 20 characters and the file was stored as 30 characters

 

DB:  asdfghjkloiuytrewqas

File: asdfghjkloiuytrewqashdulsd.jpg

 

That means the file "asdfghjkloiuytrewqas" probably doesn't exist.

[doubledee]

you could change it to empy(), but that doesn't mean that the string passed to it is a real existing file.

 

*should* and *do* are two different things, any number of errors could have gone wrong while saving the file, and the file didn't get saved, or maybe someone deleted the file without you knowing.

 

It's almost 2am here.

 

Please help me understand what you are proposing with your function.

 

 

maybe your table cell only stores 20 characters and the file was stored as 30 characters

 

DB:  asdfghjkloiuytrewqas

File: asdfghjkloiuytrewqashdulsd.jpg

 

That means the file "asdfghjkloiuytrewqas" probably doesn't exist.

 

Well, here is a snippet from my Form...

<!-- User Photo -->
<label for="userPhoto">Filename:</label>
<input id="userPhoto" name="userPhoto" type="file" />
<?php
	if (!empty($errors['upload'])){
		echo '<span class="error">' . $errors['upload'] . '</span>';
	}
?>

 

Since you get a dropdown, navigate to the file thingy, I just assumed you can't have a problem with the length of the Filename?

 

 

Plus, in my upload script, I have...

 

// ************************
// Create New Filename.		*
// ************************
$newBasename = sha1($sessMemberID . uniqid(mt_rand(), true));			//Added 2012-06-07
$newFilename = $newBasename . $fileExt;

 

Doesn't that ensure any Filename length gets converted to like 40 characters?

 

 

Debbie

 

[Drummin]

Why not use the error codes as I suggested before?  UPLOAD_ERR_OK or 0 is returned on good.

 

function validatePhoto($photoName, $photoApproved){	

// Check for Photo.
if ($_FILES['file']['error'] == UPLOAD_ERR_OK) {
//Continue other checks

[doubledee]

Why not use the error codes as I suggested before?  UPLOAD_ERR_OK or 0 is returned on good.

 

function validatePhoto($photoName, $photoApproved){	

// Check for Photo.
if ($_FILES['file']['error'] == UPLOAD_ERR_OK) {
//Continue other checks

 

Well, I suppose The Little Guy would say, "Because that doesn't ensure the File is there"

 

I haven't been able to get onto PHP.net all evening, so I can't look up file_exists()

 

Care to show me how you would implement things in my Function, The Little Guy?

 

Thanks,

 

 

Debbie

 

[The Little Guy]

This should work, it has been working for me while php.net is down/slow:

 

us2.php.net/file_exists

 

I actually take back "file_exists" and suggest "is_file" instead

[doubledee]

I actually take back "file_exists" and suggest "is_file" instead

 

Why???

 

 

Debbie

 

[doubledee]

How does is_file work?

 

How does it know where the File is at?

 

I changed my function to this, and it isn't working now...

function validatePhoto($photoName, $photoApproved){
	// Check for Photo.
	if (is_file($photoName)){
		// Photo Found.
		// Check if Approved.
		if ($photoApproved==1){
			// Photo Approved.
			$safeImage = $photoName;
		}else{
			// Photo Pending.
			$safeImage = "PhotoPendingApproval_100x76.png";
		}

	}else{
		// Photo Not Found.
		$safeImage = "NoImageAvailable_100x76.png";
	}

	return $safeImage;
}//End of validatePhoto

 

 

 

Debbie

 

[boompa]

Well, according to the documentation for is_file, the argument is the full path to the file.

 

it isn't working now...

 

It isn't working how?

[doubledee]

Well, according to the documentation for is_file, the argument is the full path to the file.

 

it isn't working now...

 

It isn't working how?

 

I am getting the Photo's Name from my database in "photo_name" and saving it to $photoName and then passing that to the Function above.

 

I don't have the file path in what is returned from my database.

 

All of my Photos are currently stored in "uploads", so I guess that is a known fact, but how should I pass that to is_file()??

 

 

Debbie

 

 

[doubledee]

Can I do something like this...

function validatePhoto($photoName, $photoApproved){
	// Check for Photo.
	if (is_file(WEB_ROOT . 'uploads/' . $photoName)){
		// Photo Found.
		// Check if Approved.
		if ($photoApproved==1){
			// Photo Approved.
			$safeImage = $photoName;
		}else{
			// Photo Pending.
			$safeImage = "PhotoPendingApproval_100x76.png";
		}

	}else{
		// Photo Not Found.
		$safeImage = "NoImageAvailable_100x76.png";
	}

	return $safeImage;
}//End of validatePhoto

 

 

Debbie

 

[The Little Guy]

yeah it should work, as long as WEB_ROOT is a correct directory path

[doubledee]

yeah it should work, as long as WEB_ROOT is a correct directory path

 

It is in my Config file and used just for such a case.

 

So, does the new Function I posted look better to you than what I originally had?

 

Is that the one I should go with?

 

 

Debbie

 

 

[The Little Guy]

This is just me, but I like to write my code like this (not needed but this is how I like to do it), otherwise your code looks fine to me:

	function validatePhoto($photoName, $photoApproved){
	// Set a default return value
	$safeImage = "NoImageAvailable_100x76.png";
	// Check for Photo.
	if (is_file(WEB_ROOT . 'uploads/' . $photoName)){
		// Photo Found.
		// Check if Approved.
		if ($photoApproved==1){
			// Photo Approved.
			$safeImage = $photoName;
		}else{
			// Photo Pending.
			$safeImage = "PhotoPendingApproval_100x76.png";
		}
	}
	return $safeImage;
}//End of validatePhoto

 

I always preset my values, it is a good habit to get into, that way you know for a fact that something will be returned (unless there is a syntax error or something). When you have more complex functions/methods, where there are loops and if/else statements; if you don't preset your values your value may not return what you expect, especially if you missed adding an else statement somewhere.

[doubledee]

This is just me, but I like to write my code like this (not needed but this is how I like to do it), otherwise your code looks fine to me:

 

Good advice.

 

Here is my updated Function...

//****************************************************************************
function validatePhoto($photoName, $photoApproved){
	/**
	 * Return Validated Photo
	 *
	 * Takes Photo Name and Photo Approved Status and returns either
	 * the Original Photo, a "Photo Pending Approval" blank,
	 * or a "No Image Available" blank.
	 *
	 * Written On: 2012-06-09
	 *
	 * @param		String		$photoName
	 * @param		Boolean		$photoApproved
	 * @return	String
	 */

	// Set Default Return Value.
	$safeImage = "NoImageAvailable_100x76.png";

	// Check for Photo.
	if (is_file(WEB_ROOT . 'uploads/' . $photoName)){
		// Photo Found.
		// Check if Approved.
		if ($photoApproved==1){
			// Photo Approved.
			$safeImage = $photoName;

		}else{
			// Photo Pending.
			$safeImage = "PhotoPendingApproval_100x76.png";
		}
	}//End of CHECK FOR PHOTO

	return $safeImage;
}//End of validatePhoto
//****************************************************************************

 

 

Thanks for all of the help!!

 

(Someday soon, I'll be "dangerous" with PHP...) 

 

 

Debbie