The If-Switch anti-pattern

reverendryan

I'll just leave this here.

function print100(){
    for($i=1; $i<101; $i++){
    
    echo $i;

   if ($i!=0 && $i %3 == 0 && $i % 5 == 0){
        $case = 0;
    }
      elseif ($i!=0 && $i % 3 == 0) {
        $case = 1;
    } elseif ($i!=0 && $i % 5 == 0){
        $case = 2;
    } else {
        $case = 3;
    }
    
    switch ($case) {
    case 0:
        echo "FizzBuzz";
        break;
    case 1:
        echo "Fizz";
        break;
    case 2:
        echo "Buzz";
        break;
    case 3:
        echo "";
        break;
}
    
  
  }
 
    
}

That's not discourse breaking the indentation, that's how he did it.

boomzilla

It separates output from logic. What's the problem?

reverendryan

Yes. Yes it does.

I'd give him credit for that if he hadn't had to google how to do the switch block.

ben_lubar

the function is called print100 but it loops from 1 to one-less-than-101 instead of 1 to 100 or 0 to one-less-than-100.

chubertdev

It uses echo instead of a Javascript alert. Hideous.

cvi

@ben_lubar said:

it loops from 1 to one-less-than-101

And checks that $i isn't zero up to three times each iteration.

Eldelshell

~~Is this JS code? Why is he using $var ?~~

Oh! It's PHP, sorry

reverendryan

No, it's PHP.

I told the candidate that he could use any language he wanted. After talking about his current project that uses a combination of Node.js and Ruby, he selected PHP.

His first step was to google how to do a loop.

chubertdev

@reverendryan said:

No, it's PHP.

I told the candidate that he could use any language he wanted. After talking about his current project that uses a combination of Node.js and Ruby, he selected PHP.

His first step was to google how to do a loop.

there's your sign...

Eldelshell

Context? Tell us he's a senior ninja dev.

reverendryan

@chubertdev said:

there's your sign...

Yeah..
@Eldelshell said:

Tell us he's a senior ninja dev.

He was applying for a junior-ish frontend position. We weren't expecting him to be amazing, but I figured he'd at least get through FizzBuzz and on to something harder. This took him 25 minutes.

Zecc

@ben_lubar said:

1 to one-less-than-101 instead of 1 to 100

Say what? Maybe I need more coffee...

flabdablet

@reverendryan said:

I figured he'd at least get through FizzBuzz and on to something harder

FizzBuzz can be pretty hard, given suitable constraints.

Keith

@Zecc said:

Say what? Maybe I need more coffee...

Mr. Lubar is commenting on the fact that the loop condition is written as $i<101 instead of $i<=100.

Eldelshell

Like making it look like a freaking RegExp?

print 'Fizz'x!($_ % 3) . 'Buzz'x!($_ % 5) || $_, "\n" for 1 .. 100;

Or using SQL?

-- Load some numbers
CREATE TABLE numbers(i INTEGER);
INSERT INTO numbers VALUES(1);
INSERT INTO numbers SELECT i + (SELECT MAX(i) FROM numbers) FROM numbers;
INSERT INTO numbers SELECT i + (SELECT MAX(i) FROM numbers) FROM numbers;
INSERT INTO numbers SELECT i + (SELECT MAX(i) FROM numbers) FROM numbers;
INSERT INTO numbers SELECT i + (SELECT MAX(i) FROM numbers) FROM numbers;
INSERT INTO numbers SELECT i + (SELECT MAX(i) FROM numbers) FROM numbers;
INSERT INTO numbers SELECT i + (SELECT MAX(i) FROM numbers) FROM numbers;
INSERT INTO numbers SELECT i + (SELECT MAX(i) FROM numbers) FROM numbers;
-- Define the fizzes and buzzes
CREATE TABLE fizzbuzz (message VARCHAR(8), divisor INTEGER);
INSERT INTO fizzbuzz VALUES('fizz',      3);
INSERT INTO fizzbuzz VALUES('buzz',      5);
INSERT INTO fizzbuzz VALUES('fizzbuzz', 15);
-- Play fizzbuzz
SELECT COALESCE(MAX(message),CAST(i AS VARCHAR(99))) AS RESULT
FROM numbers LEFT OUTER JOIN fizzbuzz ON MOD(i,divisor) = 0
GROUP BY i
HAVING i <= 100
ORDER BY i;
-- Tidy up
DROP TABLE fizzbuzz;
DROP TABLE numbers;

All code from RosettaCode.org

flabdablet

@Eldelshell said:

print 'Fizz'x!($_ % 3) . 'Buzz'x!($_ % 5) || $_, "\n" for 1 .. 100;

If you're going to use Perl, you're only allowed 48 bytes.

presidentsdaughter

I found a better if-switch pattern:


private void doSomething(String action) {
    boolean flag = Actions.A1.equals(action);

    if(flag) {
      doA1();
    }

    if(flag? false : Actions.A2.equals(action) {
      flag = true;
      doA2();
    }

    if(flag? false : Actions.A3.equals(action) {
      flag = true;
      doA3();
    }
}

Zecc

That seems like a different anti-pattern.

Quite frankly, I find it lacking in the switchness department.

Nprz

Forgetting the ugliness of the anti-pattern, etc. But isn't it supposed to print the number if it isn't fizz/buzz? It looks to just echo "".
Maybe I missed something though. Oh I did, at the top it prints the number unconditionally.
I'd fail him for correctness of the problem.

antiquarian

@reverendryan said:

He was applying for a junior-ish frontend position. We weren't expecting him to be amazing, but I figured he'd at least get through FizzBuzz and on to something harder. This took him 25 minutes.

Did you suggest that he reconsider his career choice?

[spoiler]Just kidding, you'd probably get sued for that.[/spoiler]