Weird use of "switch"



  • My buddy sent me this odd code:

            switch (true) {
                case (empty($this->options['canvas'])):
                    // Create new image from width && height of the clipping
                    $this->_img = imagecreatetruecolor(
                                $this->options['width'], $this->options['height']);
                    if (!$this->_img) {
                        return PEAR::raiseError('Could not create image canvas.');
                    }
                    break;
    
                case (is_resource($this->options['canvas']) &&
                        get_resource_type($this->options['canvas'])=='gd'):
                    // The canvas is an image resource
                    $this->_img = $this->options['canvas'];
                    break;
    
                case (is_array($this->options['canvas']) &&
                        isset($this->options['canvas']['width']) &&
                        isset($this->options['canvas']['height'])):
    
                    // Canvas must be a width and height measure
                    $this->_img = imagecreatetruecolor(
                        $this->options['canvas']['width'],
                        $this->options['canvas']['height']
                    );
                    break;
    
    
                case (is_array($this->options['canvas']) &&
                        isset($this->options['canvas']['size']) &&
                        ($this->options['canvas']['size'] = 'auto')):
    
                case (is_string($this->options['canvas']) &&
                         ($this->options['canvas'] = 'auto')):
                    $this->_mode = 'auto';
                    break;
    
                default:
                    return PEAR::raiseError('Could not create image canvas.');
    
            }
    

    He found it in the Image_Text PEAR module: http://pear.php.net/bugs/report.php?package=Image_Text

    Lines 579-620 as of version 0.6.0beta.



  • That is unusual. I can't think of why anyone would want to do that, maybe they thought it would be more readable than a bunch of if / elseif statements?     



  • I actually saw this form of a switch for the first time yesterday. It seems in PHP the "test" values of a switch statement can be dynamically computed. Thus in this case the first statement that returns true will fire. Not that I have ever used PHP, I just ran across the form in a wiki about switch statements

    Though I have no idea what happens in the case of more than one statement being true - do they all fire??

    It is weird compared to other languages, but I can see where they are coming from (Now is that TRWTF??)



  • @OzPeter said:

    Though I have no idea what happens in the case of more than one statement being true - do they all fire??

    ?php
    $a = false; $b = true; $c = true;
    
    switch (true) {
            case $a: print "hi"; break;
            case $b: print "hi";break;    
            case $c: print "hi";break;
    }
    ?
    
    hi

    Output in bold, tested with PHP 5.2.5 with Suhosin-Patch 0.9.6.2 (cli) (built: Feb 18 2008 22:57:32).



  • @Lingerance said:

    hi

    Output in bold, tested with PHP 5.2.5 with Suhosin-Patch 0.9.6.2 (cli) (built: Feb 18 2008 22:57:32).


    But which "hi" actually fired? was it b or c?


    I would assume "c", but making assumptions like that always ends in tears


    Oh .. and what happens if there is a side effect in the processing of the case statements? ie if testing for case "a" changes the results that would have ben computed in either case "b" or "c"?



  • @OzPeter said:

    But which "hi" actually fired? was it b or c?

    My bad, anyways it appears the first one fires.

    ?php
    $a = 1; $b = 0; $c = 1;
    switch (1) {
            case $a: print "a"; break;
            case $b: print "b"; break;
            case $c: print "c"; break;
    }
    ?
    
    a
    


  • I remember encountering this idiom in VB a while ago. It can be used when you would write a Select statement (e.g. you have some clearly defined cases) but the language doesn't let you, for example:

    Select Case True
      Case obj Is first_object
      ' Blah
      Case obj Is second_object
      ' Blah blah
      Case obj Is third_object
      ' Blah blah blah
      Case Else
      ' Blah blah blah blah
    End Select
    


  •  T-SQL (maybe other versions) has something similar.

    CASE WHEN X = Y

        THEN

        WHEN X = Z

        THEN

    ELSE

    END

     --- ALTERNATELY ---

    CASE X

        WHEN Z THEN

        WHEN Y THEN

        ELSE END



  • @OzPeter said:

    @Lingerance said:
    hi

    Output in bold, tested with PHP 5.2.5 with Suhosin-Patch 0.9.6.2 (cli) (built: Feb 18 2008 22:57:32).


    But which "hi" actually fired? was it b or c?

     

    Hint:

    break; statements end the switch.



  • @Lingerance said:

    @OzPeter said:
    But which "hi" actually fired? was it b or c?
    My bad, anyways it appears the first one fires.

    ?php
    $a = 1; $b = 0; $c = 1;
    switch (1) {
            case $a: print "a"; break;
            case $b: print "b"; break;
            case $c: print "c"; break;
    }
    ?
    

    a

     

    In your original code, B fires. You've changed the values of a, b and c in your second snippet.

    C(#|++) and the likes don't allow non-constants as case values? Javascript sure does.



  • I'm not sure what's so weird about this usage of switch. Granted, it's a little inverted compared to the normal way of "compare this variable's value to x or y or z", but it's clear enough.



  • Its rather sad for PHP considering that PEAR is one of the better written PHP modules.

     



  • @Jonathan Holland said:

    Its rather sad for PHP considering that PEAR is one of the better written PHP modules.

    I'm confused as to what you mean by this. 

    PEAR is not a module.  It is a repository of projects in various states of completion.

    PHP Extension and Application Repository.




  • @belgariontheking said:

    @Jonathan Holland said:

    Its rather sad for PHP considering that PEAR is one of the better written PHP modules.

    I'm confused as to what you mean by this. 

    PEAR is not a module.  It is a repository of projects in various states of completion.

    PHP Extension and Application Repository.

    And a lot of the stuff in PEAR is garbage.

     

    This is a disgusting way of implementing an if-elseif block.  It's not clear initially what it is doing, it too easy for someone to forget a break and it's just ugly.  If a developer who worked for me checked-in something like that I would break their fingers so as to spare the world from any more of their "cleverness". 



  • @morbiuswilters said:

    This is a disgusting way of implementing an if-elseif block.  It's not clear initially what it is doing, it too easy for someone to forget a break and it's just ugly.  If a developer who worked for me checked-in something like that I would break their fingers so as to spare the world from any more of their "cleverness". 

     

    I have to disagree. There are a lot of intelligent ways a switch can be "abused". This setup simply ensures that the first test that returns true will prevent all other tests from taking place. If you have some sort of caching mechanism, for instance, this could make perfect sense (pseudocode):

    switch (true) {

        case $output = get_from_memcached($query); break;

        case $output = get_from_flat_file($query); break;

        case $output = get_from_datebase($query); break;

        case $output = get_from_overly_complex_enterprisey_xml($query); break; // :-)

    }

    print $output;

    This way, the fastest method gets used (okay, it's a trivial example, I admit). The alternative is to wrap it in an otherwise obsolete function so you can use return $output as a similar break, of a bunch of nested if/else statements which in my book is most definitely less elegant and readable.

    To sum up, it's a good technique in certain circumstances. The fact that you find it confusing is probably because you don't see it in the wild often, and that in turn is due to the fact that a lot of programmers tend to suck and just use if/else because they don't know any better ;-)



  • @Monomelodies said:

    switch (true) {

        case $output = get_from_memcached($query); break;

        case $output = get_from_flat_file($query); break;

        case $output = get_from_datebase($query); break;

        case $output = get_from_overly_complex_enterprisey_xml($query); break; // :-)

    }

    print $output;

     

    You, um, forgot the condition: parts. 



  • @morbiuswilters said:

    This is a disgusting way of implementing an if-elseif block.

    I have a personal distaste for if clauses, but I don't think that's any more valid than your disgust. :)

    I make a habit of turning ifelse chains that check for different values of the same statement into switches -- but I do agree that I might turn this inverted switch into a normal ifelse.



  • @Monomelodies said:

    To sum up, it's a good technique in certain circumstances. The fact that you find it confusing is probably because you don't see it in the wild often, and that in turn is due to the fact that a lot of programmers tend to suck and just use if/else because they don't know any better ;-)
     

    I don't find it confusing but it is a needless abuse of the switch statement.  We should be striving for clarity, not cleverness.  This does nothing that an if-elseif can't do and it does it much less clearly.  If you forget a break, you get an awesome bug that's difficult to track down.  You didn't include a default so it is possible to exit the switch with $output not having been set.  Because part of the coding standards I enforce dictate that breaks must always be on their own line with the same indentation as the case, abusing a switch like this would use more lines of code than the if-elseif.  You could put a comment explaining what the code does, but if you have to add a comment to your code explaining your "elegant" usage of a standard, built-in language feature, you should not be using it to begin with.

     

    /*

    This is like an if-elseif, except way cooler.  Like, all those other idiots would just use an if-elseif, but I prefer to throw curveballs to my fellow developers!  Oh and be sure you include the break because we had to spend 9 hours tracking down the bug last time someone tried to added another case.

    */ 



  • @Monomelodies said:

    I have to disagree. There are a lot of intelligent ways a switch can be "abused".

    Yeah, this is how a lot of switch-cases were done back in the COBOL days. It was really handy because COBOL had handy little flags and you could just switch through and check them.

    TRWTF may be someone my age knowing (and liking) COBOL. 



  • @morbiuswilters said:

    Oh and be sure you include the break because we had to spend 9 hours tracking down the bug last time someone tried to added another case.

    I forget what the exact situation was, but I always love the story about how a large part of the midwest was without power because someone forgot to put the break statements in their switch-case.  I think it says more about QA than coding, though



  • @dhromed said:

    Hint:

    break; statements end the switch.

    Break statements may tell you where the code ends, but they don't tell you where the execution started.



  • @dhromed said:

    @Monomelodies said:

    switch (true) {

        case $output = get_from_memcached($query); break;

        case $output = get_from_flat_file($query); break;

        case $output = get_from_datebase($query); break;

        case $output = get_from_overly_complex_enterprisey_xml($query); break; // :-)

    }

    print $output;

     

    You, um, forgot the condition: parts. 

    Presumably, you refer to the usage of semicolons instead of colons at the end of each "case" - if you're complaining about something else, then you fail.



  • @dhromed said:

    I have a personal distaste for if clauses
    Must be tough for you.

    But yeah, I've seen this being used in VB too. 



  • Just because something can be done, it doesn't mean that it should.

    If more people learned just that one lesson, the world would be a much better, and less WTFey place.



  • @dhromed said:

    @morbiuswilters said:

    This is a disgusting way of implementing an if-elseif block.

    I have a personal distaste for if clauses, but I don't think that's any more valid than your disgust. :)

    I make a habit of turning ifelse chains that check for different values of the same statement into switches -- but I do agree that I might turn this inverted switch into a normal ifelse.

     

    I have a personal distaste for switch statements that are functionally equivalent to if-else chains. Let's hope we never work together, or a repository holy war will ensue.



  • @Nether said:

    I have a personal distaste for switch statements that are functionally equivalent to if-else chains.
    Aren't they all?

    I know, I'm not adding anything of value to this conversation. I'll shut up now. 



  • @Quietust said:

    @dhromed said:

    @Monomelodies said:

    switch (true) {

        case $output = get_from_memcached($query); break;

        case $output = get_from_flat_file($query); break;

        case $output = get_from_datebase($query); break;

        case $output = get_from_overly_complex_enterprisey_xml($query); break; // :-)

    }

    print $output;

     

    You, um, forgot the condition: parts. 

    Presumably, you refer to the usage of semicolons instead of colons at the end of each "case" - if you're complaining about something else, then you fail.

     

    Presumably, you forgot to re-read the code after  dhromed pointed out that the above snippet won't work, as it will always just execute the first case and return.

    I'd expect that the coder intended to write something more along the lines of

    [code]

    switch (true) {

        case $output = "Cache":  get_from_memcached($query); break;

        case $output = "File": get_from_flat_file($query); break;

        case $output = "DB": get_from_datebase($query); break;

        case $output = "XML": get_from_overly_complex_enterprisey_xml($query); break; // :-)

    }

    [/code] 

     

    which would actually make sense. I'd still prefer using an if/elseif here, though, as it's more readable and easier to maintain. 



  • @KenW said:

    ...a bunch of incorrect nonsense... 

    The example given by Monomelodies works properly and is how this idiom would be used.  It's still slop.  Meanwhile, your example makes no sense at all.

     

    Here's Monomelodies rewritten as an if-elseif:

    if ($output = get_from_memcached($query)) {

    } else if ($output = get_from_flat_file($query)) { 

    } else if ($output = get_from_database($query)) {
    } else if ($output = get_from_overly_complicated_enterprisey_xml($query)) {

    } else {
        echo("FAIL!\n");

        exit(0);

    }



  • @Monomelodies said:

    switch (true) {

        case $output = get_from_memcached($query); break;

        case $output = get_from_flat_file($query); break;

        case $output = get_from_datebase($query); break;

        case $output = get_from_overly_complex_enterprisey_xml($query); break; // :-)

    }

    print $output;

    The thing about this is, in a well designed application you'd never have a section of code like this. Caching is something that should be abstracted away into a get($query) method that has an internal check to see if it's already in the cache, and if not send the request down the pipeline one level. The next level down does the same. Avoiding this kind of cascading-get is why we came up with layered design in the first place.



  • @Welbog said:

    The thing about this is, in a well designed application you'd never have a section of code like this. Caching is something that should be abstracted away into a get($query) method that has an internal check to see if it's already in the cache, and if not send the request down the pipeline one level. The next level down does the same. Avoiding this kind of cascading-get is why we came up with layered design in the first place.

    What now?  How do you know this switch statement isn't in the get() method to begin with?  Obviously somewhere you have to code the logic to check the different cache levels.  It's ugly, unclear and likely to obscure bugs, which is why I prefer the if-elseif above.  Still, it's not like you'd never have to code something to handle this logic.



  • @morbiuswilters said:

    What now?  How do you know this switch statement isn't in the get() method to begin with?  Obviously somewhere you have to code the logic to check the different cache levels.  It's ugly, unclear and likely to obscure bugs, which is why I prefer the if-elseif above.  Still, it's not like you'd never have to code something to handle this logic.
    I guess it could be in the get method of the appropriate layer, but it deals with cache, database, filesystem and funky XML all in one function. To me that just jumps out as poor abstraction.



  • @Welbog said:

    I guess it could be in the get method of the appropriate layer, but it deals with cache, database, filesystem and funky XML all in one function. To me that just jumps out as poor abstraction.

    It doesn't deal with them all in one function, it deals with retrieving the data from the appropriate source all in one function.  The individual methods are encapsulated.  It's all data-level stuff, so why wouldn't you encapsulate it as such?  I really can't even fathom how you think it should work.  It has to check if the data is in the fastest cache first, then the next and so on.  Honestly, it's pretty retarded there is a flat-file and XML layer, you should really only need the database, but I believe that was only for illustrative purposes.



  • @morbiuswilters said:

    @Welbog said:

    I guess it could be in the get method of the appropriate layer, but it deals with cache, database, filesystem and funky XML all in one function. To me that just jumps out as poor abstraction.

    It doesn't deal with them all in one function, it deals with retrieving the data from the appropriate source all in one function.  The individual methods are encapsulated.  It's all data-level stuff, so why wouldn't you encapsulate it as such?  I really can't even fathom how you think it should work.  It has to check if the data is in the fastest cache first, then the next and so on.  Honestly, it's pretty retarded there is a flat-file and XML layer, you should really only need the database, but I believe that was only for illustrative purposes.

    I think he's probably complaining about it being too procedural (as opposed to all fancy and object-oriented).  He'd probably prefer something like:

    public method getValue (key) {
    unless (result = this.getValueInternal(key)) {
    result = this.getFallback().getValue(key);
    }
    return result;
    }

    where you'd write this in an abstract base class and then override getValueInternal() in the various subclasses.


Log in to reply