PHP Best Practices: Should a given parameter always have a consistent type?

Question

I have a function that accepts a checkGlossary bool param as well as an optional glossary array.
Their states are directly tied together.
The glossary is never required if the bool is FALSE, and inversely, it is always required if the bool is TRUE.

To me, it seems like this could easily be simplified from:

// Current
function doSomething($param1, $param2, $checkGlossary=FALSE, $glossary=NULL){
    // blah blah blah
    if($checkGlossary)
        array_search($glossary[$param2]);
    // etc etc etc
}

... to:

// Proposed
function doSomething($param1, $param2, $glossary=FALSE){
    // blah blah blah
    if($glossary)
        array_search($glossary[$param2]);
    // etc etc etc
}

My only hesitation is over the fact that the type for $glossary (bool or array) would be unpredictable.
It doesn't bother me as long as I'm not going against some Best-Practices guidelines.

Thoughts?

Easiest solution would be to just drop the checkGlossary flag and use only the glossary variable. Set array() as default value and ommit the check if the array is empty. — Till Helge
– Till Helge, Commented Mar 6, 2013 at 23:01
Best practices: When a method takes more than 2 arguments, time to rethink (generally). </IMO> — No Results Found
– No Results Found, Commented Mar 6, 2013 at 23:02
@TillHelgeHelwig Yours is my favorite answer to the question asked (because of specifying array() as the default value). If you put that in an answer, I'll select it. — mOrloff
– mOrloff, Commented Mar 6, 2013 at 23:08
@WesleyMurch Hmmm ... looks like I've got some refactoring to look into :) Thanks for the suggestion. — mOrloff
– mOrloff, Commented Mar 6, 2013 at 23:09
@mOrloff Okay. Posted it. Didn't have the time yesterday. :) — Till Helge
– Till Helge, Commented Mar 7, 2013 at 7:04

Till Helge · Accepted Answer · 2013-03-07 07:04:19Z

It is always a bad idea to have function parameters with what PHP calls mixed datatype. It requires additional code in the function to check the type of the parameter and obviously it can get very confusing.

Probably the easiest solution in your special case would be to use the array length as indicator of whether to use the glossary code. You need a way to declare that the glossary array should not be used. So you should ask yourself: When is it pointless to use the glossary? When it's empty of course. So, I would suggest that you get rid of the flag and define array() as default for your other parameter:

function doSomething($param1, $param2, $glossary=array()) {
    if (count($array) > 0) {
        // do your glossary code here
    }
    // all the other stuff goes here
}

To me this seems semantically correct and works just fine.

I don't know what exactly you are building there, but another solution would be to put it all into a class and have the glossary as instance variable. In case you could use the glossary in several function calls. It would roughly look like this:

 public class SomeAccurateClassName {
     private $glossary = array();

     function setGlossary(array $glossary) {
         $this->glossary = $glossary;
     }

     function doSomething($param1, $param2) {
         if (count($array) > 0) {
             // do your glossary code here
         }
         // all the other stuff goes here
     }
 }

Considering that you basically have a state (use glossary or don't use glossary) it might be a good idea to encapsulate that withint a class.

vcampitelli · Accepted Answer · 2013-03-06 23:00:38Z

2

You can use is_array() and is_bool() to check its type!

answered Mar 6, 2013 at 23:00

vcampitelli

3,2721 gold badge14 silver badges13 bronze badges

Collectives™ on Stack Overflow

PHP Best Practices: Should a given parameter always have a consistent type?

2 Answers 2

1 Comment

Comments

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

2 Answers 2

1 Comment

Comments

Your Answer

Sign up or log in

Post as a guest

Related