NYCPHP Meetup

[Daniel Convissor]

Jay:

Man, your coding style gives me a headache.  Way too much nesting, let 
alone inconsistent widths of nesting.  Then putting braces below each if 
statement makes the code take up loads of extra vertical space, so it's 
hard to see as much logic on a page.

That aside, the notion of having one exit point per function doesn't carry 
much weight in my mind.  If one reaches the end of execution, it seems 
clearest to me to actually say you've reached that point by leaving the 
function.  Otherwise, you're left wondering what else happens down in the 
rest of the function and have to go hunting, only to find out, oh, it's 
done.  What a waste.

Something I noticed in your examples is you're not handling the situation 
if $bev_type isn't coffee or tea.  I'd likely use a switch with a default 
case here.

You're assigning the returns of add_twater() and add_tea_bag() to 
variables, but the values are only used for evaluation in an if statement.  
It's more efficient to just do:

        if (!add_tea_bag($bev)) {
            return false;
        }

Now, as far as how to handle reporting of errors, that's really situation 
specific.

Here's how I'd write your function:

function make_beverage_dan($bev_type, &$bev) {
    switch ($bev_type) {
        case 'coffee':
            if (add_cwater($bev) && brew_coffee($bev)) {
                return true;
            }
            return false;

        case 'tea':
            if (add_cwater($bev) && add_tea_bag($bev)) {
                return true;
            }
            return false;

        default:
            return false;
    }
}

I notice you're passing in $bev by reference.  If that's solely to get the 
value out of the method, I tend to prefer returning the values.

--Dan

-- 
 T H E   A N A L Y S I S   A N D   S O L U T I O N S   C O M P A N Y
            data intensive web and database programming
                http://www.AnalysisAndSolutions.com/
 4015 7th Ave #4, Brooklyn NY 11232  v: 718-854-0335 f: 718-854-0409