6

I was just reading the top 100 signs of spaghetti code and I came across number 4, which simply states:

if ($status == "awake"){ 
    $actitivity = "Writing spaghetti code"; 
} else if ($healthstatus == "OK"){ 
    $activity = "Sleep"; 
} else { 
    print "CALL 911 IMMEDIATELY!"; 
}

I've seen this multiple if-else pattern in other spaghetti discussions. I'm a little confused why this is, and even if it's applicable to this example.

Is the above example bad because

  • the first variable is actitivity which indicates the coder needs some sleep, so it's a joke, or

  • view shouldn't be being output during logic, or

  • something about too many if/else

EDIT never mind this second part, it's bad because of nested conditionals and multiple returns

In the other spaghetti discussions link, is it bad because

- the logic has return in it, which breaks the flow, or

- there's too many if/else piled on top of eachother...?

Community
  • 1
  • 1
Ben
  • 54,723
  • 49
  • 178
  • 224
  • Ultimately, most of it can be boiled down to formal computer science under the banner of [Cyclomatic Complexity](http://en.wikipedia.org/wiki/Cyclomatic_complexity). – Spudley Aug 30 '13 at 08:54
  • Ahh, thanks for the link! That's a pretty clear model of what everybody was trying to describe. More nodes or changes -> more complexity -> more difficult to read -> spaghetti. – Ben Aug 30 '13 at 08:57
  • yep, that's it. But it's not just difficult to read, it's also difficult to predict. Maintenance and testing of a code block becomes exponentially harder with every additional node. – Spudley Aug 30 '13 at 09:02

1 Answers1

5

If/else statement often breaks Open-closed principle. (Java example but valid in PHP too)

Solution => favor polymorphism.

Besides, assigning a temporary variables more than once is really error prone and reduces readability. Especially in PHP since it isn't a static-typed language. Indeed, what if someone first assign $actitivity = "Writing spaghetti code"; and then $actitivity = 1; ?? ...blending apples with oranges inside the same container.. Look at this: http://sourcemaking.com/refactoring/split-temporary-variable

Also, the logic allows for side-effect (print) only if one of the condition is verified => Method isn't cohesive and thus SRP violated.

Mik378
  • 21,881
  • 15
  • 82
  • 180
  • For people like myself: `SRP states that every object should have a single responsibility, and that responsibility should be entirely encapsulated by the class`. – Ben Aug 30 '13 at 09:01
  • SRP also applies to method .. not only the whole object itself. How would you name a method that does print in one case and do other things in others case: `printPerhaps`? .. tend to make your method having a high cohesion is one of the best way to have a clean code. In this case, splitting the method. – Mik378 Aug 30 '13 at 09:02
  • 1
    For more information, look at this: http://codebork.com/2011/02/02/revisiting-solid-principles-srp-methods.html. There is one error in the second image's presented (since identical to the first one), but in the github associated, there's the good sample :) – Mik378 Aug 30 '13 at 09:08