Array input to function, a new approach

[djr33]

Alright. That's fine. And we seem to agree about the original question. (Please I don't disagree about your other points.)

[traq]

regarding your original question,

function something( $input ){
    if( is_array( $input ) ){
        foreach( $input as $item ){ /* do something  */ }
    }
    else{  /*  do something  */  }
}

#  VS.  #

function something( $input ){
    $input = (array) $input;
    foreach( $input as $item ){ /* do something  */ }
} 

…definitely #2. You don't duplicate any code, which makes everything more succinct and less error-prone. Remember, DRY.

[djr33]

Hmm... that's slightly different.

First, your code is more succinct than the examples I was looking at earlier (setting the type rather than checking if it's an array and if so doing something special).

Second, I don't see why there's any "repeating" going on with either example from the first post, actually. They may not be optimized, but I don't think it's due to repeating in any sense that would matter.

function something( $input ){
    $input = (array) $input;
    foreach( $input as $item ){ /* do something  */ }
} 

That looks very clean, I admit. But it still seems awkward and undesirable to me. Two reasons:
1. This function is about strings (or ints, etc.), not about arrays. Using everything as an array is backwards, though it works.
2. For reasons of tabs and at least apparent simplicity, wrapping the contents of the entire function in a foreach loop (it might be 1000 lines, who knows) seems undesirable to me. Keeping the actual purpose of the function at a less indented level (literally and figuratively) just makes more sense to me. Here's an example:

//here's the simple version:
function notify($email) {
   mail($email,'Notification: standard message.');
}

//but now we might want to sometimes allow an array as input, rather than in many places using a foreach loop for that (and violating the "DRY" idea above

//traq's idea:
function notify($email) {
   $email = (array) $email;
   foreach($email as $e) {
      mail($e,'Notification: standard message');
   }
}

//my idea:
function notify($email) {
   if (is_array($email)) { //ah, an array, let's get rid of this
      foreach($email as $e) {
         notify($e);
      }
      return 'array'; //or whatever you want there
   }
   else if (!is_string($email)) { //uh-oh, bad input, don't even start running the core of the function
      return false;
   }
   //pre-processing done, moving on to core of the function

   //do whatever you want, no complications:
   mail($email,'Notification: standard message');
} 

Clearly my version looks like the original function more. It just involves an exception at the top, and you can also include your type/security checking there (in my "else if" block) rather than embedding it somewhere below, awkwardly, in the foreach. Seems preferable to me. It's not the shortest code, nor necessarily the fastest to process, though, I admit.



Interestingly, I notice that my version is actually recursive which is potentially convenient if you have multi-level arrays. I don't know that it would occur often, but you could just combine a few different arrays together and they'd all run smoothly through that. With a single layered foreach loop that wouldn't work.




(You're right about needing an "else" in my original code, example #2. Thanks. I might actually use a "return" statement instead, but the same idea basically.)





--

The more I think about it, Nile's original point might be right. It's very hard to come up with a convincing example of when this would really be the best way to design a function. But it's certainly a common practice from what I've seen...


There's also a very weird result here where you can't conveniently have a return value for the whole process. Using arrays in this case is a little awkward actually. Convenient to a degree I suppose.

[Nile]

I do agree with @traq that injecting the array is probably the way to go.

Interestingly, I notice that my version is actually recursive which is potentially convenient if you have multi-level arrays. I don't know that it would occur often, but you could just combine a few different arrays together and they'd all run smoothly through that. With a single layered foreach loop that wouldn't work.

I noticed this too. Interesting.

The more I think about it, Nile's original point might be right. It's very hard to come up with a convincing example of when this would really be the best way to design a function. But it's certainly a common practice from what I've seen...

I'm obligated to agree . Of course we always need to remember that tons of "common practice(s)" we see are complete violations of general rules (or principles*) of programming.

In my opinion, depending on how convoluted those functions get, what we're really looking at in the examples are violations of separation of concerns. Ideally, if this is a complicated process, there should be something dealing with collections and something dealing with individual instances. After all, you're right that:

This function is about strings (or ints, etc.), not about arrays. Using everything as an array is backwards, though it works.

So to separate the concerns, we should just give them different methods/controls/classes/etc.. (of course, there are situational choices to make). Also, note that I'm not talking about SRP (single responsibility principle). I do agree that these are very similar responsibilities (situationally, again, this could change), but if these are genuine concerns dealt with by jamming multiple functionalities into one function, separation needs to take place.

*Making this clarification because I once told a friend they were violating the single responsibility principle. His response was "there are no rules in programming." :facepalm:

[djr33]

Right. The whole thing is collapsing what should be, perhaps, two functions, one for strings, and another for passing array items as strings to that first function. So myfunction($string) and myfunction_forarrays($array). Basically what I have above, but not shoved into a single function. Maybe that actually is the real answer. Don't take shortcuts then it's silly to do so. But that would mean remembering which function takes only strings and which one takes only arrays when referring to them later.

[Nile]

Don't take shortcuts then it's silly to do so. But that would mean remembering which function takes only strings and which one takes only arrays when referring to them later.

Consistency beats this one, of course. I'm positive I've seen users name multiple patterns different things in their code, and it's awful. If I'm going to make multiple functions that deal with arrays in the same way by calling the instance-based function, I'm definitely going to use the same naming patterns.

[djr33]

Ah. Well, naming should be consistent (though it often isn't, especially when multiple designers are involved in an open source project). But I just meant in general that you'd need to remember that there are two functions and when to use each, rather than just assuming the basic (string) one always works. It wouldn't be hard if you did that consistently, but it would require some planning ahead. A good thing, probably.

[traq]

recursion is a good alternative, too.

Right. The whole thing is collapsing what should be, perhaps, two functions, one for strings, and another for passing array items as strings to that first function. So myfunction($string) and myfunction_forarrays($array). Basically what I have above, but not shoved into a single function. Maybe that actually is the real answer. Don't take shortcuts then it's silly to do so. But that would mean remembering which function takes only strings and which one takes only arrays when referring to them later.

I disagree. It's better in a single function because it's all in the same place and is easier to understand and manage. When this single function becomes too large or complex, it should become a class (or a controlling function that delegates to each type-specific function).

[djr33]

I disagree. It's better in a single function because it's all in the same place and is easier to understand and manage. When this single function becomes too large or complex, it should become a class (or a controlling function that delegates to each type-specific function).

Really, though, that's a violation of DRY-- every function like this would need that same extra code. I suppose there's no good solution
And there are many solutions that work.