SitePoint Sponsor

User Tag List

Results 1 to 16 of 16
  1. #1
    messing with my mind fristi's Avatar
    Join Date
    Feb 2009
    Posts
    292
    Mentioned
    0 Post(s)
    Tagged
    0 Thread(s)

    Method Arguments vs more methods

    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
    To PHP or to Perl, that is the question!
    (Bucket - simpletest) User

  2. #2
    SitePoint Wizard silver trophy kyberfabrikken's Avatar
    Join Date
    Jun 2004
    Location
    Copenhagen, Denmark
    Posts
    6,157
    Mentioned
    0 Post(s)
    Tagged
    0 Thread(s)
    I would much prefer the two-functions approach, because it makes code more readable. When you see the client code, it's obvious. Compare:

    PHP Code:
    $foo->setFile('blah'42'stuff'true); 
    and
    PHP Code:
    $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.

  3. #3
    SitePoint Addict Mastodont's Avatar
    Join Date
    Mar 2007
    Location
    Czech Republic
    Posts
    375
    Mentioned
    0 Post(s)
    Tagged
    0 Thread(s)
    There is also third setup:
    PHP Code:
    function SetFile() {
       switch 
    func_num_args() {
       ......
       }

    It has to be used carefully, though.

  4. #4
    SitePoint Wizard bronze trophy
    Join Date
    Jul 2006
    Location
    Augusta, Georgia, United States
    Posts
    4,141
    Mentioned
    16 Post(s)
    Tagged
    3 Thread(s)
    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.

    PHP Code:
    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;
        }



  5. #5
    SitePoint Guru
    Join Date
    Jun 2006
    Posts
    638
    Mentioned
    0 Post(s)
    Tagged
    0 Thread(s)
    What's wrong with:
    PHP Code:
    // $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'truefalse); 
    The only time you would want to overload this function would be for type hinting, ex:
    PHP Code:
    $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)

  6. #6
    SitePoint Guru
    Join Date
    May 2005
    Location
    Finland
    Posts
    608
    Mentioned
    0 Post(s)
    Tagged
    0 Thread(s)
    Quote Originally Posted by Vali View Post
    What's wrong with:
    PHP Code:
    $foo->setFile('blah'42'stuff'); 
    $foo->setFile('blah'42'stuff'true); 
    $foo->setFile('blah'42'stuff'truefalse); 
    What are the parameters for? You can't tell, it's opaque. With a fluent API, they become obvious.
    PHP Code:
    $file->setName('blah')
        ->
    setMaxSize(42)
        ->
    setTextBody('stuff')
        ->
    setFileLock(true)
        ->
    setSpecialCase(false); 

  7. #7
    SitePoint Guru
    Join Date
    Jun 2006
    Posts
    638
    Mentioned
    0 Post(s)
    Tagged
    0 Thread(s)
    Quote Originally Posted by Ezku View Post
    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.

  8. #8
    Spirit Coder allspiritseve's Avatar
    Join Date
    Dec 2002
    Location
    Ann Arbor, MI (USA)
    Posts
    648
    Mentioned
    0 Post(s)
    Tagged
    0 Thread(s)
    Quote Originally Posted by Vali View Post
    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.

  9. #9
    messing with my mind fristi's Avatar
    Join Date
    Feb 2009
    Posts
    292
    Mentioned
    0 Post(s)
    Tagged
    0 Thread(s)
    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 Code:
    <?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:

    Quote Originally Posted by oddz View Post
    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
    Last edited by fristi; May 25, 2009 at 23:52. Reason: added quote
    To PHP or to Perl, that is the question!
    (Bucket - simpletest) User

  10. #10
    . shoooo... silver trophy logic_earth's Avatar
    Join Date
    Oct 2005
    Location
    CA
    Posts
    9,013
    Mentioned
    8 Post(s)
    Tagged
    0 Thread(s)
    Quote Originally Posted by fristi View Post
    There is also the way of making boolean arguments more readable like:
    PHP Code:
    <?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.

    PHP Code:
    class TestThis
    {
        const
            
    ONE_CONST 1,
            
    TWO_CONST 2;
        
        protected
            
    $one_var 1,
            
    $two_var 2;

    Logic without the fatal effects.
    All code snippets are licensed under WTFPL.


  11. #11
    messing with my mind fristi's Avatar
    Join Date
    Feb 2009
    Posts
    292
    Mentioned
    0 Post(s)
    Tagged
    0 Thread(s)
    Quote Originally Posted by logic_earth View Post
    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 Code:
    <?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
    To PHP or to Perl, that is the question!
    (Bucket - simpletest) User

  12. #12
    Theoretical Physics Student bronze trophy Jake Arkinstall's Avatar
    Join Date
    May 2006
    Location
    Lancaster University, UK
    Posts
    7,062
    Mentioned
    2 Post(s)
    Tagged
    0 Thread(s)
    Well, out of the examples given so far, Ezku's is by far the easiest to understand.

    It would be implemented like:
    PHP Code:
    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); 
    Last edited by Jake Arkinstall; May 26, 2009 at 07:00.
    Jake Arkinstall
    "Sometimes you don't need to reinvent the wheel;
    Sometimes its enough to make that wheel more rounded"-Molona

  13. #13
    SitePoint Wizard silver trophy kyberfabrikken's Avatar
    Join Date
    Jun 2004
    Location
    Copenhagen, Denmark
    Posts
    6,157
    Mentioned
    0 Post(s)
    Tagged
    0 Thread(s)
    Quote Originally Posted by arkinstall View Post
    PHP Code:
        public function enableFileLock(){
            return 
    setFileLock(true);
        } 
    Been coding much Java lately, have you?

  14. #14
    Theoretical Physics Student bronze trophy Jake Arkinstall's Avatar
    Join Date
    May 2006
    Location
    Lancaster University, UK
    Posts
    7,062
    Mentioned
    2 Post(s)
    Tagged
    0 Thread(s)
    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.
    Jake Arkinstall
    "Sometimes you don't need to reinvent the wheel;
    Sometimes its enough to make that wheel more rounded"-Molona

  15. #15
    SitePoint Wizard silver trophy kyberfabrikken's Avatar
    Join Date
    Jun 2004
    Location
    Copenhagen, Denmark
    Posts
    6,157
    Mentioned
    0 Post(s)
    Tagged
    0 Thread(s)
    I meant .. You forget $this->

  16. #16
    Theoretical Physics Student bronze trophy Jake Arkinstall's Avatar
    Join Date
    May 2006
    Location
    Lancaster University, UK
    Posts
    7,062
    Mentioned
    2 Post(s)
    Tagged
    0 Thread(s)
    Ah, ok then!

    Will update it now. Damn, I really need to pay more attention!
    Jake Arkinstall
    "Sometimes you don't need to reinvent the wheel;
    Sometimes its enough to make that wheel more rounded"-Molona


Bookmarks

Posting Permissions

  • You may not post new threads
  • You may not post replies
  • You may not post attachments
  • You may not edit your posts
  •