• Tim Gallagher (cs)
    Tim Gallagher:

    T. M. write, "I'm working with a new PHP based startup run by a few fellows who absolutely hate templating engines. I came across a bit of code that exemplifies this...

    This is funny to me personally because I just wrote an article on PHP templating.

  • Lummox (unregistered)
    Tim Gallagher:

    Andrew Zhilenko writes, "A new developer was given a task to modify one of the custom modules in our [ticketing system]. When a new ticket arrives during the business hours, it should be randomly assigned to either one of the two available sales persons. The job was reported as being completed, but then something was not working quite right. Investigation showed that the code was as follows:

    if ($hour>=7 and $hour<18) {
    # Biff or Cliff
    if (rand() > 0.5) {$Owner->LoadById('70656');}
    if (rand() < 0.5) {$Owner->LoadById('72586');}
    }

    I could just see the conversation:

     "70656, why are you so behind in your tickets?  I just talked to 72586 and he finished all of his before lunch!"

    (Yes, the numbers sound awkward, but it was the best I could do to represent the salepeople :P)

    The other thing that comes to mind is, does a ticket give you permission?  If so, did 70656 have any influence on the code? :P
     

     

  • Lummox (unregistered) in reply to Lummox

    Doh,

    -permission

    +commission

     It's late and I'm not thinking while I'm typing :/
     

  • DW (unregistered) in reply to Lummox

     

    Who gets the ticket when Rand draws 0.5 :)

  • John (unregistered) in reply to Lummox

    Well,  the behavior would depend on what language that's in wouldn't it? If it's Perl (which I believe it could be), 72586 could have twice as much work as 70656 (depending on how the system handles multiple owners.

  • rbs (unregistered) in reply to DW

    So approximately 1/4 of all tickets just dropped down a rathole somewhere?

  • tin (cs) in reply to Lummox
    Anonymous:

    I could just see the conversation:

     "70656, why are you so behind in your tickets?  I just talked to 72586 and he finished all of his before lunch!"

    (Yes, the numbers sound awkward, but it was the best I could do to represent the salepeople :P)

    The other thing that comes to mind is, does a ticket give you permission?  If so, did 70656 have any influence on the code? :P
     

     

    Oh dear... I didn't even notice the hugeness of that WTF until you made fun of it. I guess neither of them get the ticket if rand returns exactly 0.5 twice in a row....

  • brendan (unregistered)

    The first one Has a posible recursion problem (it gives a new meaning to the word panic).

    second one, I really see no problem with except that the whole function is pointless.

    The thrid one it is posible that both will get it the job and even sometimes nun of the them would get the job.

  • rbs (unregistered) in reply to brendan
    Anonymous:

    The thrid one it is posible that both will get it the job and even sometimes nun of the them would get the job.


     

    No, it's two calls to rand.

     

     There's a 50% chance that 70656 gets the ticket.

     

     Of the other tickets, there's a 50% chance that 72586 gets it.

     

    All other tickets are left unassigned.

     

     

  • yv (unregistered) in reply to brendan
    Anonymous:
    pointless.

    The thrid one it is posible that both will get it the job and even sometimes nun of the them would get the job.


    They have nuns working on the helpdesk now? Cool... :)
  • Anon (unregistered) in reply to tin

    rand() is being called 2 times, if the first call returns a value less than 0.5 and the second returns a value greater than 0.5, the ticket is not assigned to anyone.

  • brendan (unregistered) in reply to rbs
    Comment held for moderation.
  • brendan (unregistered) in reply to brendan

    it's possible for both of them to get the job if the first call returns >0.5 and the second call returns < 0.5

    and BTW 

    $A = rand();

      if ($A > 0.5) {$Owner->LoadById('70656');}
     $A = rand();
    if (rand() < 0.5) {$Owner->LoadById('72586');}
    is equal to

    $A = rand();

    if ($A > 0.5) {$Owner->LoadById('70656');}

    $A = rand();
    if ($A < 0.5) {$Owner->LoadById('72586');}

     

     

  • cdr (unregistered) in reply to Tim Gallagher
    Comment held for moderation.
  • RiX0R (cs) in reply to brendan

    Anonymous:
    The thrid one it is posible that both will get it the job and even sometimes nun of the them would get the job.

     That depends on the system, but it's probable that only the owner that was assigned last gets the actual ticket; so it's not really possible for both to get the ticket. It is, however, possible, that none get the ticket.

    The following code is correct, but illustrates why the previous code is wrong (the initialization doesn't always happen):

    # Biff or Cliff
    $Owner->LoadById('70656');
    if (rand() < 0.5) {$Owner->LoadById('72586');}

  • Anonymous (unregistered) in reply to rbs
    Anonymous:
    There's a 50% chance that 70656 gets the ticket.

     

     Of the other tickets, there's a 50% chance that 72586 gets it.

     

    All other tickets are left unassigned.

    That would be the case if it was an if/else-if pair. However, it's two if's, so what you said there is incorrect. There's a 50% chance the first guy gets the ticket, and there's a 50% chance the second guy gets the ticket. Hence, altogether, there's a 25% chance that both of them claim the ticket, 25% that neither, and a 50% chance that only one of them will claim the ticket.

  • Hairy Monkey (unregistered) in reply to RiX0R
    RiX0R:

    Anonymous:
    The thrid one it is posible that both will get it the job and even sometimes nun of the them would get the job.

     That depends on the system, but it's probable that only the owner that was assigned last gets the actual ticket; so it's not really possible for both to get the ticket. It is, however, possible, that none get the ticket.

    The following code is correct, but illustrates why the previous code is wrong (the initialization doesn't always happen):

    # Biff or Cliff
    $Owner->LoadById('70656');
    if (rand() < 0.5) {$Owner->LoadById('72586');}

    The real WTF is that apparently no one has ever heard of 'else'. Not knowing what those functions do, the above could well assign the ticket always to 70656 and in addition half the tickets to the other guy.

    Captcha = null, which the above code does not test either, nor FileNotFound, but is it less than 0.5?

     

  • fauxparse (unregistered) in reply to John

    Anonymous:
    Well,  the behavior would depend on what language that's in wouldn't it? If it's Perl (which I believe it could be), 72586 could have twice as much work as 70656 (depending on how the system handles multiple owners.


    Nah, it's clearly one of those languages without else statements.

  • rbs (unregistered) in reply to brendan
    Anonymous:

    no, it does something simular to this

     

    Oooh, my bad. I was thinking I saw a "else if".

    That'll teach me to drink and comment.

    But at least I wasn't drinking and programming.
     

  • angus (unregistered) in reply to Tim Gallagher
    Comment held for moderation.
  • Martin (unregistered) in reply to Tim Gallagher
    Comment held for moderation.
  • RiX0R (cs) in reply to Hairy Monkey

    You're obviously a troll but here goes anyway:

    Anonymous:

    The real WTF is that apparently no one has ever heard of 'else'. Not knowing what those functions do, the above could well assign the ticket always to 70656 and in addition half the tickets to the other guy.

    Yes well: while presenting that code I explicitly stated the assumption that the ticket would only be assigned to the last owner loaded. I challenge you to find fault with the code under that assumption.

    Futhermore, I have two reasons to suspect the assumption is correct:

    1. The code doesn't look to assign anything... $owner->loadById()... this simply loads something, and doesn't assign anything. An assign action would include both an owner and ticket entity. Okay so it's obfuscated code, so that's not really strong.

    2. If tickets actually got assigned to two people, odds are the bug would have been discovered much earlier as Biff and Cliff noticed they both owned the same ticket. On the other hand, lost tickets are quite a lot harder to discover.

    Finally, the use of "else" is not the issue here. Sure it should have been used in the original code, but I only posted the above code to illustrate the bug more clearly.

  • cheesy (cs) in reply to Hairy Monkey
    Anonymous:
    RiX0R:

    Anonymous:
    The thrid one it is posible that both will get it the job and even sometimes nun of the them would get the job.

     That depends on the system, but it's probable that only the owner that was assigned last gets the actual ticket; so it's not really possible for both to get the ticket. It is, however, possible, that none get the ticket.

    The following code is correct, but illustrates why the previous code is wrong (the initialization doesn't always happen):

    # Biff or Cliff
    $Owner->LoadById('70656');
    if (rand() < 0.5) {$Owner->LoadById('72586');}

    The real WTF is that apparently no one has ever heard of 'else'. Not knowing what those functions do, the above could well assign the ticket always to 70656 and in addition half the tickets to the other guy.

    Captcha = null, which the above code does not test either, nor FileNotFound, but is it less than 0.5?

     


    The use of "else" wouldn't be required, although it would be better style.

    if (rand() > 0.5)
        // assign to dude #1
    else
        // assign to dude #2

    ...would be ideal, but this would work too, which seems to be what the original author meant to do (although slightly less efficient)

    a = rand();
    if (a > 0.5)
        // assign to dude #1
    if (a <= 0.5)
        // assign to dude #2

    Gotta use "less than or equal" as to not skip cases when rand happens to product exactly 0.5.
  • Vector (cs)

    Good haul for a CodeSOD. :)

    As for the random allocation...

    if ( ($hour >= 7) && ($hour <= 18) )
    {
        $id = ( rand() > 0.5 ) ? 70656 : 72586;
        $Owner->LoadByID( $id );
    }

    Done. 

  • anonymous (unregistered)
    Tim Gallagher:
    function br($count=1) {
    $br = '
    ';
    $out = '';
    for($i=0;$i<$count;$i++) {
    $out .= $br;
    }
    return $out;
    }
     This can be less efficient for the fun factor:
     function br($count=1) {
       if (!$count) return "<!-- br -->"; 
    return "<script>for(t=0;t<$count;t++) document.write("<br>");</script>";
    }
     
  • Ahox (unregistered) in reply to rbs

    Anonymous:

    Anonymous:

    The thrid one it is posible that both will get it the job and even sometimes nun of the them would get the job.


     

    No, it's two calls to rand.

     There's a 50% chance that 70656 gets the ticket.

     Of the other tickets, there's a 50% chance that 72586 gets it.

    All other tickets are left unassigned.

     

    Actually no,

    there is a 25% chance for 70656 and a 50% chance for the second and in a quarter of all cases no one gets it.

  • Ahox (unregistered) in reply to cheesy
    cheesy:
    The use of "else" wouldn't be required, although it would be better style.

    if (rand() > 0.5)
        // assign to dude #1
    else
        // assign to dude #2

    ...would be ideal, but this would work too, which seems to be what the original author meant to do (although slightly less efficient)

    a = rand();
    if (a > 0.5)
        // assign to dude #1
    if (a <= 0.5)
        // assign to dude #2

    Gotta use "less than or equal" as to not skip cases when rand happens to product exactly 0.5.

     Thats just not fair, in this case dude #2 would have to work more. Whats about

    do{
    $a=rand();
    }while( a!=0.5)
    $owner =getById( a>0.5? 1 : 2 )  // is this kind of conditional assignment legal in Perl?

     

  • Anonymous Bastard (unregistered)

    I get it. The code was originally written in haskell, and since no functions can produce side effects, calling rand() twice will yield the same result. Still doesn't explain what happens on a perfect half.

  • maht (unregistered) in reply to Tim Gallagher
    Comment held for moderation.
  • Dr_Barnowl (cs)

    My first thought too, was that 25% of the tickets were vanishing into the aether. So that's a WTF all on its lonesome, but there is another one....

     
    If this is PHP code, 70656 will get the vast majority of the tickets. In PHP, rand() returns an integer between 0 and RAND_MAX (on win32, this is 2^15 -1). About 1 in 32,000 tickets will disappear into the aether.

     
    Lucky/poor old 72586 will only get one out of every billion or so tickets, as to give him a ticket, rand() has to return 0 twice in a row. While this should be technically possible, I'm willing to bet that the nature of psuedo-random number generator code means that he actually gets none.

  • xix (unregistered) in reply to Dr_Barnowl
    Dr_Barnowl:

    My first thought too, was that 25% of the tickets were vanishing into the aether. So that's a WTF all on its lonesome, but there is another one....

     
    If this is PHP code, 70656 will get the vast majority of the tickets. In PHP, rand() returns an integer between 0 and RAND_MAX (on win32, this is 2^15 -1). About 1 in 32,000 tickets will disappear into the aether.

     
    Lucky/poor old 72586 will only get one out of every billion or so tickets, as to give him a ticket, rand() has to return 0 twice in a row. While this should be technically possible, I'm willing to bet that the nature of psuedo-random number generator code means that he actually gets none.

    Naw, looks like Perl, with the # comment. And rand() in that case does go from 0 to 1. Not that there aren't enough WTF's there already

    captcha: mustache, ironic for reasons I won't go into

  • Dr_Barnowl (cs) in reply to xix

    I thought I might have got that wrong, as I'm not experienced enough with Perl or PHP to distinguish them ; but having just checked, the PHP manual says that it does support Perl style comments (and a brief test shows it to be true), so it's entirely plausible that this could be PHP code.

     I'm also under the impression that PHP is more popular than perl, so it's even more likely....
     

  • Alien426 (unregistered)
    Comment held for moderation.
  • Skroog (cs) in reply to Lummox
    Anonymous:
    Tim Gallagher:

    Andrew Zhilenko writes, "A new developer was given a task to modify one of the custom modules in our [ticketing system]. When a new ticket arrives during the business hours, it should be randomly assigned to either one of the two available sales persons. The job was reported as being completed, but then something was not working quite right. Investigation showed that the code was as follows:

    if ($hour>=7 and $hour<18) {
    # Biff or Cliff
    if (rand() > 0.5) {$Owner->LoadById('70656');}
    if (rand() < 0.5) {$Owner->LoadById('72586');}
    }

    I could just see the conversation:

     "70656, why are you so behind in your tickets?  I just talked to 72586 and he finished all of his before lunch!"

    (Yes, the numbers sound awkward, but it was the best I could do to represent the salepeople :P)

    The other thing that comes to mind is, does a ticket give you permission?  If so, did 70656 have any influence on the code? :P
     

     

     

    Um.... why don't you just call them Biff and Cliff in your conversation? :P

  • Anonymous (unregistered)

    The first example is just great. I'd like to disconnect the database server or anything like that from time to time just to screw up the clients.
     

    If snipped #3 is actually php, all tickets will be assigned to '70656' as rand(); in php doesn't return a float but a int from 0 to system specific maximum. But maybe it got simplified by the sender.

     BTW: I'm curious about what you guys would call the best php templating strategy if plain php files and smarty both suck badly.

     

    Captcha: mustache -  smells more like dirty sanchez

  • Steinar H. Gunderson (unregistered) in reply to Alien426

    Dude, this is not PHP. :-)

    (From the looks of it, I'd guess this is code from a Request Tracker module; RT is written in Perl.)

    To consider your points one by one:

    1. Yes, that is indeed the WTF.
    2. The chances of hitting a specific float value from rand() is about as close to zero as you can get.
    3. In Perl, the result is not an integer; by default it's a random number from [0,1>.
    4. See #3.

     

  • zamies (cs) in reply to Lummox
    Anonymous:
    Tim Gallagher:

    Andrew Zhilenko writes, "A new developer was given a task to modify one of the custom modules in our [ticketing system]. When a new ticket arrives during the business hours, it should be randomly assigned to either one of the two available sales persons. The job was reported as being completed, but then something was not working quite right. Investigation showed that the code was as follows:

    if ($hour>=7 and $hour<18) {
    # Biff or Cliff
    if (rand() > 0.5) {$Owner->LoadById('70656');}
    if (rand() < 0.5) {$Owner->LoadById('72586');}
    }

    I could just see the conversation:

     "70656, why are you so behind in your tickets?  I just talked to 72586 and he finished all of his before lunch!"

    (Yes, the numbers sound awkward, but it was the best I could do to represent the salepeople :P)

    The other thing that comes to mind is, does a ticket give you permission?  If so, did 70656 have any influence on the code? :P 

     IMHO you're missing the point here.

    It makes two separate rand() calls, so tickets most probably often don't get assigned.

     

  • javascript jan (unregistered) in reply to Tim Gallagher
    Comment held for moderation.
  • CNU (unregistered) in reply to xix
    Anonymous:
    Dr_Barnowl:

    My first thought too, was that 25% of the tickets were vanishing into the aether. So that's a WTF all on its lonesome, but there is another one....

     
    If this is PHP code, 70656 will get the vast majority of the tickets. In PHP, rand() returns an integer between 0 and RAND_MAX (on win32, this is 2^15 -1). About 1 in 32,000 tickets will disappear into the aether.

     
    Lucky/poor old 72586 will only get one out of every billion or so tickets, as to give him a ticket, rand() has to return 0 twice in a row. While this should be technically possible, I'm willing to bet that the nature of psuedo-random number generator code means that he actually gets none.

    Naw, looks like Perl, with the # comment. And rand() in that case does go from 0 to 1. Not that there aren't enough WTF's there already

    captcha: mustache, ironic for reasons I won't go into

  • mrsticks1982 (cs) in reply to Alien426
    Anonymous:
    Tim Gallagher:
    if ($hour>=7 and $hour<18) {
    # Biff or Cliff
    if (rand() > 0.5) {$Owner->LoadById('70656');}
    if (rand() < 0.5) {$Owner->LoadById('72586');}
    }

    OK, let's analyze once more:

    1. rand() gets called twice, can result in two different values that get checked.

    2. If this got fixed with an assignment to a variable, what if rand() would return 0.5?

    3. It will never return 0.5 since its result is an integer (http://de.php.net/manual/en/function.rand.php)

    4. Without arguments the function returns an integer between 0 and RAND_MAX (32767 on my system).

    So if you fix nothing, user 70656 will always get the ticket.

     

     

    http://www.perlmeme.org/howtos/perlfunc/rand_function.html 

  • SpComb (cs) in reply to javascript jan
    Anonymous:

     The system you describe is just PHP; in particular, you describe content pages as pulling in formatting (headers, footers, etc). Typically when people talk about "templating engines" they are referring to a top-down organisation where _content_ is pulled in and requires no cargo-cult boilerplate.

     
    The real WTF is that I fell for your obvious self-pimping :-)

    template file: 

    <html>
     <head>
      <title><?=$page_title;?></title>
     </head>
     <body>
    <? $body(); ?>
     </body>
    </html>
    php code: 
    
       class PageDisplayer implements Renderable {
                private $path;
                public function __construct ($path) {
                        $this-&gt;path = $path;
                }
                public function render() {
                        readfile($this-&gt;path);
                }
        }
        $body = new PageDisplayer(&quot;pages/&quot; . $page-&gt;filename . &quot;.page&quot;);
        $layout = Template::create(&#39;layout&#39;);
        $layout-&gt;bind(&#39;page_title&#39;, &quot;Random Page :: &quot; . $page-&gt;title);
        $layout-&gt;bind_tpl(&#39;body&#39;, $body);
        $layout-&gt;render();</pre><pre>disclaimer: example code written several months ago, I have since been introducted to python &lt;3&nbsp;</pre>
    
  • emurphy (cs)

    I suppose

     

    use define SALESPERSON1_ID => 70656; # Biff

    use define SALESPERSON2_ID => 72586; # Cliff


    is considered too obvious to bother pointing out explicitly?

     

  • Anon (unregistered)

    Even if the logic around rand() was correct - WTF are they randomly assigning for anyway?

    Imagine if the tickets take an hour for a sales guy to process and they only get 10 or so a day...

  • Martin (unregistered)
    Tim Gallagher:

    Andrew Zhilenko writes, "A new developer was given a task to modify one of the custom modules in our [ticketing system]. When a new ticket arrives during the business hours, it should be randomly assigned to either one of the two available sales persons. The job was reported as being completed, but then something was not working quite right. Investigation showed that the code was as follows:

    if ($hour>=7 and $hour<18) {
    # Biff or Cliff
    if (rand() > 0.5) {$Owner->LoadById('70656');}
    if (rand() < 0.5) {$Owner->LoadById('72586');}
    }

    * By gently I mean, "Let's get the rakes and pitchforks out and go!"

     

    The realest WTF is that the ids were hard coded yielding a fun to maintain bit when there's  a staffing change. He should have had the ticket assigned to a random person in the sales group. . .and of course, if there's no sales group, or the ticketting system doesn't support it then clearly those things must be added.

    ** Martin 

  • Sandy (unregistered) in reply to javascript jan

    "The system you describe is just PHP"
     

    Well, PHP is a templating language. Otherwise those pesky <?php tags wouldn't be necessary.

    Any "templating engine" powerful enough to handle the needs of most complex web apps will end up reimplementing most of PHP anyway, so the way to protect yourself against mixing logic and presentation is to become a better programmer/hire better programmers, not put blind faith in the fact you've put another layer of templating on top of your templating engine.
     

  • mol (unregistered) in reply to Anonymous
    Anonymous:

     BTW: I'm curious about what you guys would call the best php templating strategy if plain php files and smarty both suck badly.

    Since PHP 5: XSLT  

  • UncleMidriff (cs) in reply to Martin

    Is there any reason to be randomly assigning tickets?  It could very well happen, even with a perfectly random ticket assigning process, that, out of 100 tickets, Person #1 could get 1 ticket, Person #2 could get 5 tickets, and Person #3 could get 94 tickets.  That'd be unlikely, I suppose, but it could happen.  Ideally, shouldn't such a system, upon receiving a new ticket to assign, check to see who has how many tickets, and then assign the new ticket to the guy with the fewest tickets already?  I've never worked on a ticketing system before, so I'm speaking out of ignorance here, but it seems to me that randomly assigning tickets is an odd way to go about things.

  • Tim Gallagher (cs) in reply to Martin
    Anonymous:
    Tim Gallagher:

    This is funny to me personally because I just wrote an article on PHP templating.

    This kind of so called "PHP templating", you wrote about in your article, should be avoided in any case! It is the same s..t as Smarty!

    I'm really curious why you feel this way.  Can you give me some well-reasoned points to support your argument? 

  • Alex Brick (unregistered)
    Tim Gallagher:
    Andrew Zhilenko writes, "A new developer was given a task to modify one of the custom modules in our [ticketing system]. When a new ticket arrives during the business hours, it should be randomly assigned to either one of the two available sales persons. The job was reported as being completed, but then something was not working quite right. Investigation showed that the code was as follows:
    if ($hour>=7 and $hour<18) {
    # Biff or Cliff
    if (rand() > 0.5) {$Owner->LoadById('70656');}
    if (rand() < 0.5) {$Owner->LoadById('72586');}
    }
    While I was just looking at this code, it occurred to me that if this is the situation I think it is, it's even more wrong.  Let's say that this is in a subroutine that returns the owner of the ticket (which would make sense with the lack of assignment).  In Perl, the last statement executed is the return value of a subroutine, but here, the last statement could be the comparison in the second if statement if the if statement is false. 
    Wow. 
    captcha: jiggles 
  • Huggie (unregistered) in reply to DW
    Anonymous:

    Who gets the ticket when Rand draws 0.5 :)

    You're missing half the humor.  I actually thought the same thing when I first looked at it, but it's even worse than that.  Notice that the "rand()" function itself is being called in both spots rather than utilization of a "rand" variable.  This will cause two different random values to be generated.  As such, it might be < 0.5 for the > 0.5 test (meaning it won't get assigned to the first person), then it might be > 0.5 for the < 0.5 test (meaning it won't get assigned to the second person).

     

    This is going to happen even more often than what you thought about the equals case! :)

Leave a comment on “Triple Play”

Log In or post as a guest

Replying to comment #:

« Return to Article