Conditional modifier syntax

Ruby
1

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 :-/

2

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”

PS: It’s beautiful but it doesn’t work :-/

Well, you can use if in two different ways:

a if b

or

if a
b
end

Only in the second case you can use else. I would write it this way:
(in two lines, in case that doesn’t show in the mail)

raise TypeError “The playlist must be an instance of XSPF::Playlist”
unless source[:playlist].instance_of?(XSPF::Playlist)
@playlist= source[:playlist]

I tend to find that more clear: you first leave out the case that
doesn’t interest you.

Just my opinion :wink: !

Vince
3

Pau Garcia i Quiles wrote:

PS: It’s beautiful but it doesn’t work :-/

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.

David V.