David Vallner
10/23/2006 12:12:00 AM
Pau Garcia i Quiles wrote:
> Hello,
>
> Am I the only one who finds beautiful this syntax for conditional modifiers?
>
> @playlist= source[:playlist] if source[:playlist].instance_of?(XSPF::Playlist)
> else raise TypeError "The playlist must be an instance of XSPF::Playlist"
>
> It reads like a book even if you don't know Ruby: "@playlist is X if this or Y
> if not"
>
> PS: It's beautiful but it doesn't work :-/
>
<rant>
I wouldn't like else in conditional modifiers for the same reason I
don't like conditional modifiers after code blocks (the not-a-closure kind).
I prefer to use them for trivial special case handling (early returns,
loop cycle breaks) when it's more important to see what's happening than
why. (I find it easier to visually parse past a block of "return nil if
some_condition" on the beginning of a method to get to the main logic,
for example.)
A conditional modifier effectively "hides" the conditional; when used
with a code block, it's cheating people reading your code into believing
there's one level of logic flow nesting less (it's not immediately
visible as indentation, for example). There's a word for this: obfuscation.
The above code snippet also doesn't read as you describe it, instead it
reads "@playlist is X if X is a Playlist, fail if not." You place the
failure in the source after the assignment, when in fact you're checking
against a precondition. (Emphasis on pre-). The method fails -before-
the assignment, placing the cause of the failure after it is misleading.
I'd rewrite the code snippet as:
raise TypeError "The playlist must be an instance of XSPF::Playlist"
unless source[:playlist].is_a? XSPF::Playlist
@playlist = source[:playlist]
grouping the special cases (precondition checking) at the beginning of
the method to ensure that any failure of those is clean and doesn't
leave the object in an inconsistent state.
</rant>
David Vallner