Should all functions return something?

Not for 'how-to' coding questions but PHP theory instead, this forum is here for those of us who wish to learn about design aspects of programming with PHP.

Moderator: General Moderators

timvw
DevNet Master
Posts: 4897
Joined: Mon Jan 19, 2004 11:11 pm
Location: Leuven, Belgium

Post by timvw »

In cases where you expect a number you could indicated an error by returing true/false.
In cases where you don't expect a number you could return 0/1 to indicated an error.
As a consequence you'll end up with an API that has two sorts of values to indicate an error...

With exception handling there is only one sort of result (the actual result) and in case something went wrong an exception will be thrown. This way you don't have to clutter your API with all sorts of return vaues that have a special meaning. A consequence of exceptions is that a function can be interrupted halfway and will not return a result.

If all you want to know if the function was successful (or not) handling exceptions only complicates the code. Instead of simply getting back a result, you also have to handle eventual exceptions. Luckily enough this handling is pretty simple, because it always means a failure...
User avatar
theFool
Forum Newbie
Posts: 17
Joined: Thu Oct 26, 2006 2:00 am
Location: Berlin, DE

Post by theFool »

k. i'll make it simple

i have a function 'login'.

i'll return 0 if there is no user found
i'll return 0 if you couldn't be logged in
i'll return 1 if it worked
i'll return -1 if you are already logged in
also, i use -1 for errors where things shouldn't happen.
I don't like magic numbers, mostly because I can'T remember them after a couple of days by myself... ^^
how could I expect more from someone else :roll:
User avatar
MrPotatoes
Forum Regular
Posts: 617
Joined: Wed May 24, 2006 6:42 am

Post by MrPotatoes »

*ugh*

i can see that trying to talk it through is absolutely useless and i give up. you win. congrats
User avatar
Jenk
DevNet Master
Posts: 3587
Joined: Mon Sep 19, 2005 6:24 am
Location: London

Post by Jenk »

Similar to what I posted in a different thread, Exceptions are thrown when the function/method experiences an exceptional circumstance, or result. I.e. a circumstance or result that is not expected as normal behaviour.

This has been established from languages such as C/C++ and Java where we are very restricted with return types - i.e. they are defined before we can even use the function, so upon finding a circumstance that is not what we expect, the function/method throws an exception as the only clear way of declaring an error has occured. (Yes, there is a difference between an error and an exception, before anyone picks me about it :p, but that's a different topic.)

PHP in it's current form is loosely typed, but through demand from the user base, exceptions have found their way in, and by the looks of it, so will return type-hinting.

So if you have a function, say arithmetic, defined to return integers: Using 1/0/-1 for success/fail/other will not be viable.. we can't use true/false as we are bound by the integer type hint, so we must throw an exception to indicate the operation has stepped out of expected results.
AlexC
Forum Commoner
Posts: 83
Joined: Mon May 22, 2006 10:03 am

Post by AlexC »

Hum I didn't seem to get any emails from this topic even though I set it to email me!

Maybe I should re-phrase my question.

I normally code by starting a class from an included file, like.......hum Section 2 of the Session module from a CMS I am coding, here is the class code:

Code: Select all

// Sector 2 - Log user in
	class modSession_sec2 {
	
		var $redirectURL;

		function modSession_sec2() {

			global $libsession, $libmysql;
				
			$User = $libmysql->smartClean( $_POST['user'] );
			$Pass = $libmysql->smartClean( $_POST['pass'] );			
			
			// Build the URL to redirect to
			if ( $_SESSION['rsec'] != NULL ) {
				$section = '&sec='.$_SESSION['rsec'];
			}
			
			if ( $_SESSION['rmod'] != NULL ) {
				$mod = '?mod='.$_SESSION['rmod'];
			}
			
			// Reset Session Values
			$_SESSION['rmod'] = NULL;
			$_SESSION['rsec'] = NULL;
			
			$this->redirectURL = 'index.php'.$mod.$section;
			 
			$this->CheckDetails( $User, $Pass );			
			
		}
		
		// Check if Details are valid 
		function CheckDetails( $User, $Pass ) {
			
			global $libmysql, $liblog;
			
			$Query = $libmysql->Query( " SELECT * FROM tcm_users WHERE username = '$User' AND password = md5('$Pass') " );
			
			if ( mysql_num_rows( $Query ) < 1 ) {
			
				// Log the failed attempt
				$liblog->LogEvent( 0 );
				
				// Details are not Valid
				$_SESSION['error'] = 12;
				
			} else if ( $this->CheckPermission( $User ) == 1 ) {
				// Log the user in
				$this->UserLogin( $User );
			} else {
				// Log the failed attempt
				$liblog->LogEvent( 0 );	
				$_SESSION['error'] = 13;						
			}
			
			Redirect( $this->redirectURL );	
									
		}
		
				
		// Check if user has permission to login
		function CheckPermission( $User ) {
		
			global $libmysql, $liblog;
			
			// Find what group number the user is in
			$Query = $libmysql->Query( " SELECT * FROM tcm_users WHERE username = '$User' " );
			$Result = mysql_fetch_array( $Query );
			
			$adminAccess = $Result['adminAccess'];			
			$UserGroup = $Result['group'];
			
			// Find the _real_ group ( IE, the Name )
			$Query = $libmysql->Query( " SELECT * FROM tcm_user_groups WHERE id = '$UserGroup' " );
			$Result = mysql_fetch_array( $Query );

			if ( $adminAccess == 1 ) {
				// User can login
				return 1;
			} else {
				// User can't login
				$liblog->LogEvent( 0 );
				return 0;
			}		
		
		}
		
		// Log the user in
		function UserLogin( $User ) {
		
			global $libmysql, $liblog, $libsession;

			$SID = session_id();
			
			// Insert the SessionID into database
			$Query = $libmysql->Query( " UPDATE tcm_users SET sid = '$SID' WHERE username = '$User' " );
			$Query = $libmysql->Query( " SELECT `id` FROM `tcm_users` WHERE `username` = '$User' " );
			$row = mysql_fetch_array( $Query );
		
			// Update Session Cookies
			$_SESSION['user'] = $row['id'];	

			$libsession->UserInfo();
			
			// Log user logging in
			$liblog->logEvent( 1 );
			
			redirect( $this->redirectURL );				
		
		}	
	
	}
Ignore the actual code for now, it's quite old and well crap anyway! - but you can see the way I am coding. Instead of returning values I just call the next function that I need - this is what I am unsure about, should I be doing this? I use the functions as a container pretty much.
User avatar
Christopher
Site Administrator
Posts: 13596
Joined: Wed Aug 25, 2004 7:54 pm
Location: New York, NY, US

Post by Christopher »

I don't see a problem with redirecting like you are, especially if you are processing a login form. However you may want to move to controllers with a Response object -- that might give you more control over whether to redirect or forward.
(#10850)
AlexC
Forum Commoner
Posts: 83
Joined: Mon May 22, 2006 10:03 am

Post by AlexC »

Well it's not just that piece of code I do it in, arborint, that is just how I code ( Classes with functions calling other functions ). Instead of doing:

Code: Select all

<?php

require_once( 'libtest.php' );
$libtest = new libtest();

$libtest->thisFunction();
$libtest->thatFunction();
$libtest->nextFunction();

?>
my code would start of with a Constructor class in libtest.php - once the code in the constructor function has finished, it would then call thatfunction() from within the constructor function, then thatfunction() would call nextfunction() etc.
User avatar
RobertGonzalez
Site Administrator
Posts: 14293
Joined: Tue Sep 09, 2003 6:04 pm
Location: Fremont, CA, USA

Post by RobertGonzalez »

As it relates to returning something from a function, I use a return in almost all functions/methods. Whether it is a null return, a return value or return status. it just makes things easier for me. And I try to stay away from magic numbers. I will, however, try to use constant if I need to use numbers that range more than 0 and 1. But that is just me.
User avatar
Christopher
Site Administrator
Posts: 13596
Joined: Wed Aug 25, 2004 7:54 pm
Location: New York, NY, US

Post by Christopher »

AlexC_ wrote:Well it's not just that piece of code I do it in, arborint, that is just how I code ( Classes with functions calling other functions ). Instead of doing:

my code would start of with a Constructor class in libtest.php - once the code in the constructor function has finished, it would then call thatfunction() from within the constructor function, then thatfunction() would call nextfunction() etc.
Yeah ... I see the code where, for example, the constructor calls CheckDetails() when it could all be one big method. And you are using globals. Those are all just choices. You are dealing with the effects of those choices and as long as you are ok with your choices the you're fine.

I guess I am wondering what you are looking for? Going back to your OP (before the Potato detour) your semi-question was
AlexC_ wrote: I'm pretty sure I should not be coding like this, is it bad that I do? Should all my functions be returning something and instead of just doing:
I guess I really don't see return values as the problem. But, I think you will find most folks in PHP forums a little too weary to try to convince yet-another-programmer to not use globals or write Transaction Scripts (you might try McGruff over at Sitepoint). ;)
(#10850)
User avatar
Jenk
DevNet Master
Posts: 3587
Joined: Mon Sep 19, 2005 6:24 am
Location: London

Post by Jenk »

YAP.. hmm. I like that.
User avatar
RobertGonzalez
Site Administrator
Posts: 14293
Joined: Tue Sep 09, 2003 6:04 pm
Location: Fremont, CA, USA

Post by RobertGonzalez »

arborint wrote:But, I think you will find most folks in PHP forums a little too weary to try to convince yet-another-programmer to not use globals or write Transaction Scripts (you might try McGruff over at Sitepoint). ;)
Nice... :lol:
Post Reply