Author Topic: Shit code that gets in when you don't have code reviews  (Read 3720 times)

Mike

  • Jackass In Charge
  • Posts: 11248
  • Karma: +168/-32
  • Ex Asshole - a better and more caring person.
Shit code that gets in when you don't have code reviews
« on: December 11, 2014, 11:11:35 PM »
I found this gem today (reduced down just a little):
Code: [Select]
<?php

switch (true) {
    case (
$perm->CanRead('permission1') || $perm->CanRead('permission2')):
       break;

    case (
$some_other_condition_expression):
        break;

    case (
$one_last_expression):
        break;
}

I was floored.  I was amazed that (and sickened) that first PHP allowed that and second that someone actually thought it was a good idea to use it.  And to top it off, we found that the second two cases were never going to be true and were completely useless.

hans

  • Guitar Addict
  • Jackass In Charge
  • Posts: 3523
  • Karma: +46/-18
Re: Shit code that gets in when you don't have code reviews
« Reply #1 on: December 11, 2014, 11:34:38 PM »
I've never seen that technique. Interesting.
This signature intentionally left blank.

Mike

  • Jackass In Charge
  • Posts: 11248
  • Karma: +168/-32
  • Ex Asshole - a better and more caring person.
Re: Shit code that gets in when you don't have code reviews
« Reply #2 on: December 12, 2014, 12:30:56 AM »
I hope to never see it again.

micah

  • A real person, on the Internet.
  • Ass Wipe
  • Posts: 6915
  • Karma: +58/-55
  • Truth cannot contradict truth.
    • micahj.com
Re: Shit code that gets in when you don't have code reviews
« Reply #3 on: December 12, 2014, 12:49:02 AM »
I mean, it kinda makes sense if you're just trying to test which conditions true first in a given order.
"I possess a device, in my pocket, that is capable of accessing the entirety of information known to man.  I use it to look at pictures of cats and get in arguments with strangers."

Mike

  • Jackass In Charge
  • Posts: 11248
  • Karma: +168/-32
  • Ex Asshole - a better and more caring person.
Re: Shit code that gets in when you don't have code reviews
« Reply #4 on: December 12, 2014, 12:55:51 AM »
Been looking around and it appears to be common(ish).

My issue is that it isn't immediately and intuitively obvious what the behavior will be.  I'd go with the if..elseif..else chain, especially in cases where you aren't using case fall through.

micah

  • A real person, on the Internet.
  • Ass Wipe
  • Posts: 6915
  • Karma: +58/-55
  • Truth cannot contradict truth.
    • micahj.com
Re: Shit code that gets in when you don't have code reviews
« Reply #5 on: December 12, 2014, 08:37:30 AM »
Yeah, it does seem weird.  Was there a default case or is it impossible for everything to be false?
"I possess a device, in my pocket, that is capable of accessing the entirety of information known to man.  I use it to look at pictures of cats and get in arguments with strangers."

Mike

  • Jackass In Charge
  • Posts: 11248
  • Karma: +168/-32
  • Ex Asshole - a better and more caring person.
Re: Shit code that gets in when you don't have code reviews
« Reply #6 on: December 12, 2014, 09:24:17 AM »
No default, cases could all be false.  The one that was found was adding additional where clasuses to the query to restrict it further.