GOTO considered harmful

[304NotModified]

GOTO considered harmful

[304NotModified]

Related #689

[logiclrd]

Huh. :-) goto case isn't quite the same thing as goto per se, and the scope here is quite clearly visible on one page on the screen, but it works either way. :-)

[logiclrd]

(Put another way, switch is effectively a giant goto that just doesn't have goto in the name, and goto case is a part of that flow structure, scoped to the switch block. If goto truly is harmful, then switch is harmful, and conversely if switch is not harmful, then flowing from one case to another ought not to be harmful either. That's why I personally have no qualms with writing a default case as a goto case for the default behaviour.)

[304NotModified]

goto case isn't a keyword, so it's a goto. I disagree that switch is a giant goto. A switch is a selection statement like if-else.
If switch was harmful, then if-else was also harmful. Because if-else isn't and because goto is harmful, switch isn't like a giant goto statement.

I don't see any reason to use a goto in the switch and therefor removal was the best choice.

[logiclrd]

But that's exactly the case. :-) if-else, switch and goto are all fundamentally the same thing, and I'm not just talking pedantics of the underlying machine code.

The negative attention that goto has received is actually a misattribution. Many people believe there was a popular article by a famous programmer penned with that title. This is actually not quite correct, and opinions on goto by such accomplished programmers is far from polarized. There's a good collection of insights here:

http://stackoverflow.com/questions/46586/goto-still-considered-harmful

See quotes of Dijkstra himself, Knuth and, further down, Torvalds. Most pointedly, the title "GOTO considered harmful" was added by the editor, replacing Knuth's much more conservative "A case against the goto statement".

The way I think about it is, if it makes the code clearer, then I use it, otherwise I don't. In this case, I feel that it makes explicit the situation where a caller has given AlignmentOnTruncation an unexpected value. The switch block as originally written is saying, basically:

switch ()
{
  case FirstValueISupport:
    ...;
    break;
  case SecondValueISupport:
    ...
    break;

  case WhatOnEarthIsThis:
    goto case AReasonableFallbackBehaviour;
}

This makes a statement about what we do if, for instance, someone adds enumerated value PaddingHorizontalAlignment.Center or PaddingHorizontalAlignment.Invalid and this code hasn't been updated to handle it, or perhaps if the user casts some random integer to the PaddingHorizontalAlignment type and it has no interpretation. (Can happen by accident with some mapping methodologies, e.g.)

The updated form handles unrecognized values in the same way, but the behaviour is implicit. There's nothing that calls the reader's attention to the fact that there might be unrecognized values in AlignmentOnTruncation. I don't anticipate that there ever would be unrecognized values in that particular field of this particular class, but programming intentionally and explicitly is a habit I've gotten into. :-)

Finally, while it is true that goto case is not a keyword in and of itself, it is also significant that the scope of target labels when using goto case *identifier* or goto default is more strict than that of the generalized goto *label*.

To be clear, I've put way more time and effort into writing this up than I have consternation about the change. I'm not at all upset by this follow-up pull request. I do disagree with the principle behind it, but ultimately the code still does the same thing, and the codebase is yours to style as you see fit. i'll not pass up an opportunity for some friendly information-passing on the subject, though. :-)