Method Arguments vs more methods

[fristi]

Hi,

what are your preferences, what would you recommend and why?

For example:

SetFile($Arg1, $Arg2, $Arg3, $Optional);

Or the other setup:

SetFile($Arg1, $Arg2, $Arg3);

SetOptionalFile($Arg1, $Arg2, $Arg3) {

if (!empty()) $this -> SetFile($Arg1, $Arg2, $Arg3);
Return TRUE;
}


Thanks

[kyberfabrikken]

I would much prefer the two-functions approach, because it makes code more readable. When you see the client code, it's obvious. Compare:

$foo->setFile('blah', 42, 'stuff', true); 

and

$foo->SetOptionalFile('blah', 42, 'stuff'); 

In general, boolean arguments to functions are often an indicator that you should refactor it out into two separate functions.

[Mastodont]

There is also third setup:

function SetFile() {
   switch func_num_args() {
   ......
   }
} 

It has to be used carefully, though.

[oddz]

Assuming that the arguments represent properties of a file I would create a class to represent the file. Then I would use individual setters for each property of the file. This would be separate from the class that handles the file object. From your example that seems like what your trying to accomplish. On a generic level the decision can't be made without knowing the purpose of the method(s). I always make this decision based on scope and purpose.

class File {

    protected $mime;
    protected $name;
    protected $temp;

    public function  setName($name) {
        $this->name = $name;
    }
    
    public function  setMime($mime) {
        $this->mime = $mime;
    }
    
    public function setTemp($temp) {
        $this->temp = $temp;
    }

}


class FileManager {

    protected $file;
    
    public function  setFile(File $file) {
        $this->file = $file;
    }

} 

[Vali]

What's wrong with:

// $foo->setFile( file_name, max_size, text_body, [file lock, default false]. [special case, default false] ); 
$foo->setFile('blah', 42, 'stuff'); 
$foo->setFile('blah', 42, 'stuff', true); 
$foo->setFile('blah', 42, 'stuff', true, false); 

The only time you would want to overload this function would be for type hinting, ex:

$myFile = new SpecialFile();
// $foo->setFile(SpecialFile $file)
$foo->setFile($myFile); 

And in this case you can rename the function (so your code it's simpler to read)

[Ezku]

What's wrong with:

$foo->setFile('blah', 42, 'stuff'); 
$foo->setFile('blah', 42, 'stuff', true); 
$foo->setFile('blah', 42, 'stuff', true, false); 

What are the parameters for? You can't tell, it's opaque. With a fluent API, they become obvious.

$file->setName('blah')
    ->setMaxSize(42)
    ->setTextBody('stuff')
    ->setFileLock(true)
    ->setSpecialCase(false); 

[Vali]

What are the parameters for? You can't tell, it's opaque. With a fluent API, they become obvious.

Your not going to use setters/getters for every function you make in your project just so you can tell what they are for.

There are 2 ways to know what they are:
#1 search the documentation (yes, i know, i hate it also)
#2 use a good IDE that searches the documentation for you, and as you type, it will tell you. Some good IDEs will tell you what the parameters are from your own comments (custom functions), and even go to that function's declaration if you control+click that function.

[allspiritseve]

There are 2 ways to know what they are:
#1 search the documentation (yes, i know, i hate it also)
#2 use a good IDE that searches the documentation for you, and as you type, it will tell you. Some good IDEs will tell you what the parameters are from your own comments (custom functions), and even go to that function's declaration if you control+click that function.

You're assuming the documentation is up to date and in sync with the code. I'd much rather have readable code that says what it does than incomprehensible code and superb documentation. It just makes things easier, both for the developer and for the user.

[fristi]

The reason why I came up with this question is because I was going through some old code of mine. I used to cram as much possibilities into one function.
Which ended up in a few boolean arguments... But I thought that was the way to go, showing what I could do...
Going through that code again afterwards I had to go and search my function lists to figure out what exactly was going on in there. That's why I suddenly started doubting. Code I wrote less than a year ago had become annoyingly unreadable.

Apparently the struggle in my own brain is also a point of discussion between people and from what I can see it really comes down to personal preference and there is no general rule towards this.

There is also the way of making boolean arguments more readable like:

<?php
    define('OPTIONAL', TRUE);
    $foo -> setfile('blah', 42, 'stuff', OPTIONAL);
?>

But the problem with this approach is that there will be a lot of define statements to 'dirty up' your code.

Thanks for all the reactions people



EDIT:

Assuming that the arguments represent properties of a file I would create a class to represent the file. Then I would use individual setters for each property of the file. This would be separate from the class that handles the file object. From your example that seems like what your trying to accomplish. On a generic level the decision can't be made without knowing the purpose of the method(s). I always make this decision based on scope and purpose.

It is actually a function in my form handling class to upload file uploads

[logic_earth]

There is also the way of making boolean arguments more readable like:

<?php
    define('OPTIONAL', TRUE);
    $foo -> setfile('blah', 42, 'stuff', OPTIONAL);
?>

But the problem with this approach is that there will be a lot of define statements to 'dirty up' your code.

Well you don't have to 'dirty up' your code with define statements.
If you are using objects and PHP 5 can use Class Constants.

class TestThis
{
    const
        ONE_CONST = 1,
        TWO_CONST = 2;
    
    protected
        $one_var = 1,
        $two_var = 2;
} 

[fristi]

Well you don't have to 'dirty up' your code with define statements.
If you are using objects and PHP 5 can use Class Constants.

Yes, But that only works when using the methods in another class right?
If I'm to use this in a "main" script, the only way to go is to use define.
Or am I wrong?
Using:

<?php
$foo -> setfile('blah', 42, 'stuff', GlobalClass::OPTIONAL); 
?>

Seems not so pure either.

But please correct me if I'm wrong, I'm still learning

[Jake Arkinstall]

Well, out of the examples given so far, Ezku's is by far the easiest to understand.

It would be implemented like:

class File{
    protected $Name = ''
                , $MaxSize = 64
                , $TextBody = ''
                , $FileLock = false
                , $SpecialCase = false
    ;
    public function setName($Name){ $this->Name = $Name; return $this;}
    public function getName(){ return $this->Name; }
    public function setMaxSize($MaxSize){ $this->MaxSize = (int)$MaxSize; return $this; }
    public function getMaxSize(){ return $this->MaxSize; }
    public function setTextBody($TextBody){ $this->TextBody = $TextBody; return $this; }
    public function getTextBody(){ return $this->TextBody; }
    public function setFileLock($FileLock){ $this->FileLock = (bool)$FileLock; return $this; }
    public function isFileLock(){ return $this->FileLock; }
    public function setSpecialCase($SpecialCase){ $this->SpecialCase = (bool)$SpecialCase; }
    public function isSpecialCase(){ return $this->SpecialCase; }
    public function enableFileLock(){
        return $this->setFileLock(true);
    }
    public function disableFileLock(){
        return $this->setFileLock(false);
    }
    public function enableSpecialCase(){
        return $this->setSpecialCase(true);
    }
    public function disableSpecialCase(){
        return $this->setSpecialCase(false);
    }
}
$File = new File();
$File->setName('blah')
      ->setMaxSize(42)
      ->setTextBody('stuff')
      ->enableFileLock()
      ->disableSpecialCase();
$Foo->SetFile($File); 

[kyberfabrikken]

    public function enableFileLock(){
        return setFileLock(true);
    } 

Been coding much Java lately, have you?

[Jake Arkinstall]

Actually, no! ( ) I've been doing quite a bit of C++, and because of the way that you lay methods out (simply listing them in the class definition and actually defining them in a source file), it's tempting to add more and more methods for common functionality. I suppose previous Java programming may have had some role there.

In fact, it's been very useful. It's amazing how much quicker writing applications is once you have written methods for many common tasks.

Of course, the enableFileLock/disableFileLock is only useful when it's certain what you want to do, which is why the other setFileLock is available for when you want to set it depending on an outcome.

[kyberfabrikken]

I meant .. You forget $this->

[Jake Arkinstall]

Ah, ok then!

Will update it now. Damn, I really need to pay more attention!