views:

211

answers:

5

I'm using someone elses class and that person has defined a function with five arguments.

in Sentry.php:

function checkLogin($user = '',$pass = '',$group = 10,$goodRedirect = '',$badRedirect = '')

If all five fields are filled in this leads to a login procedure.

Now on the page where he explains how to use this there is a snippet which, according to php.net, doesn't make sense.

in a page that loads the sentry:

require_once('../system/Sentry.php');
$theSentry = new Sentry();
if(!$theSentry->checkLogin(2)){ header("Location: login.php"); die(); }

which by default should behave as such that it checks if the $group argument is <= 10 (default). In this situation it should be two. If the user checked has a group-variable of <=2 this should enable the person to view the page.

However, this doesn't work and for a very obvious reason: the php manual states:

Note that when using default arguments, any defaults should be on the right side of any non-default arguments; otherwise, things will not work as expected.

So the code, according to phpbuilder.com should have no optional ($variable = default_something) field to fill in with a call to the function and it definitely shouldn't be defined as the third of five arguments.

How can I use the function like this?:

checkLogin(2)
A: 
checkLogin(NULL,NULL,2);

but honestly bad coding style.

Rufinus
so changing the order of the arguments is the best bet?
xaddict
i would say a checkLogin method without a username and password can't check much :)so die first two params (username/password) should be mandatory).but this is true only in this specific checkLogin case.
Rufinus
Giving NULL as arguments doesn't make the function use it's default argument value, so in this case $user and $pass are NULL and not '', which probably doesn't make any difference in this case.
danamlund
if you take it by the letter you are right. im just so used to null from zend framework :)
Rufinus
@Rufinus: about your second comment; the username and password are taken care of by the function. When nothing is filled in it checks for session variables autmatically. if other stuff is filled in it rechecks if the current user has any privileges and sets the session variables accordingly.
xaddict
A: 

The easiest way is just to change the order of arguments so that $group is first.

da5id
+6  A: 

Default arguments are PHP's way of dealing with the lack of overloaded functions. In Java you can write this:

public void login(String username)
public void login(String username, String password)

In PHP you have to solve it like this:

function login($username, $password = '')

This way $username is mandatory and $password is optional. While this may be convenient, it isn't always. In your example there are a log of arguments and they are all optional. A clean solution for this would be to make 1 function that does the work and add 'convenience' methods to make the interface cleaner.

So add:

function checkLoginGroup($group = 10) {
  $this->checkLogin('', '', $group);
}

It simply calls the function that already exists, but it allows for a cleaner interface, just call:

$theSentry->checkLoginGroup(2);

It's even neater to make the provided method private (or protected, depending on your needs) and create public 'convience' methods so you can hide the implementation details.

However, if you can not or don't want to change the original class, you might be able to create a subclass.

Mythica
+1  A: 

Changing the order of the arguments is a possibility... But what for the time you'll need another argument to be "more optionnal" or "less optionnal" ? Will you change that order again ?
And, with it, every call that's made to the function ?

To pass only the third argument, considering your function's declaration, the only method I see is to give the three first arguments... Passing the default value (which you'll have to know, of course -- just look at it's declaration, for that) for the first two :

checkLogin('', '', 20);

In my opinion, using this instead of that one :

checkLogin(null, null, 20);

Has the advantage of being explicit : someone looking at the function's declaration will immediatly notice you're using the default values for the first two parameters.

Using NULL, the personn reading your code would have to check inside the function's code to see if NULL is handled in a special way (it could, afterall !) ; it wouldn't be as easy for someone to understand that this is your way of passing default values... As they are not the default values ^^


Other solutions would imply refactoring the function,

Either way, you'd lose the ability your IDE has to show you the parameters the function is waiting for ; and that's bad :-(
And you'd also lose the phpdoc...

Pascal MARTIN
NULL is not the same as the default values.
hobodave
I agreee ; that's why I said he should use '' (the default values), and not "NULL"
Pascal MARTIN
+1  A: 

In addition to Mythicas excellent answer, I just want to point out another way of dealing with a lot of optional arguments: Arrays.

function checkLogin($args) {
    $defaults = array('user' => null, 'pass' => null, 'group' => null, ...);
    $args = array_merge($defaults, $args);
    ...
}

checkLogin(array('user' => 'x', 'group' => 9));

This essentially bypasses PHPs optional/required argument syntax entirely and instead custom-handles these things internally. It has the advantage of avoiding the problem you describe and making the function call extremely readable. The problem is that it makes the function more complex and you can't take advantage of PHP checking the existence of arguments for you.

deceze