assignment in conditional warning

[Rasputin]

Why in the world is there a warning, *even with brackets*, for assignments
in conditionals.

Most novice programmers would want to use that.

Ruby is not Python.

I'm just glad that I can turn them off. They just get more and more
annoying with each incrementing version.

 

[Robert Klemme]

Why in the world is there a warning, *even with brackets*, for assignments
in conditionals.

There are not always warnings; it seems, it's only warned if the right side
is a literal (i.e. a string or a number). This is reasonable because it is
likely that a comparison was meant. After all, what do you gain by doing

if ( x = 10 )
end

over

if ( 10 )
end

?

Most novice programmers would want to use that.

I beg to differ: the *only* reasonable usage of assignment in conditionals
is with "while" and "until", i.e. looping constructs, like in:

while ( line = gets )
line.chomp!
# do something with current line
end

Assignment in "if" and "unless" is totally superfluous. It obfuscates code
and is not needed at all. Every "if ( x = expression )" can be converted to

x = expression
if x
....

which is *much* clearer and cleaner IMHO. Alternatively you can do this in
some cases:

x = expression and puts "yes"

Ruby is not Python.
True.

I'm just glad that I can turn them off. They just get more and more
annoying with each incrementing version.

You should not turn them off but rather change the style of coding. I can't
even tell when I saw that waning last time (other than for some experiments
for this thread). And I don't have these warnings switched off.

Kind regards

robert

 

[Ara.T.Howard]

I beg to differ: the *only* reasonable usage of assignment in conditionals
is with "while" and "until", i.e. looping constructs, like in:

while ( line = gets )
line.chomp!
# do something with current line
end

Assignment in "if" and "unless" is totally superfluous. It obfuscates code
and is not needed at all. Every "if ( x = expression )" can be converted to

x = expression
if x
...

which is *much* clearer and cleaner IMHO. Alternatively you can do this in
some cases:

x = expression and puts "yes"

IMHO the above fails apart when you are testing compound datastructures using
assignment to deconstruct the structure into more manageable peices in the
test. which is cleaner/clearer?

1)

tuples = db.execute sql

raise "something terrible has happened in the database" unless
(tuple = tuples.first) and (answer = tuple.first) and (answer == 42)

2)

tuples = db.execute sql

raise "something terrible has happened in the database" unless
tuples.first and tuples.first.first and tuples.first.first == 42

3)

tuples = db.execute sql

tuple = tuples.first
answer = tuple.first

raise "something terrible has happened in the database" unless
answer == 42



i can seem someone chosing 1 or 3 from above. but when the code size starts
to grow i think most programmers, given a choice between two clear statements,
chose the shorter one and that, in fact, the __length__ of a statement
relative to it's source file is directly related to it's understand-ability.
especially consider the case where the variables 'tuple' and 'answer' are not
used except for testing purposes - isn't it better to isolate them to the
conditional expression then? if course, you don't have to name them at all
(2), but that IS obfuscated!

cheers.

-a
--
===============================================================================
| EMAIL :: Ara [dot] T [dot] Howard [at] noaa [dot] gov
| PHONE :: 303.497.6469
| A flower falls, even though we love it; and a weed grows, even though we do
| not love it. --Dogen
===============================================================================

 

[Robert Klemme]

IMHO the above fails apart when you are testing compound datastructures using
assignment to deconstruct the structure into more manageable peices in the
test. which is cleaner/clearer?

1)

tuples = db.execute sql

raise "something terrible has happened in the database" unless
(tuple = tuples.first) and (answer = tuple.first) and (answer == 42)

2)

tuples = db.execute sql

raise "something terrible has happened in the database" unless
tuples.first and tuples.first.first and tuples.first.first == 42

3)

tuples = db.execute sql

tuple = tuples.first
answer = tuple.first

raise "something terrible has happened in the database" unless
answer == 42



i can seem someone chosing 1 or 3 from above. but when the code size starts
to grow i think most programmers, given a choice between two clear statements,
chose the shorter one and that, in fact, the __length__ of a statement
relative to it's source file is directly related to it's understand-ability.
especially consider the case where the variables 'tuple' and 'answer' are not
used except for testing purposes - isn't it better to isolate them to the
conditional expression then? if course, you don't have to name them at all
(2), but that IS obfuscated!

One of the three tests is superfluous. So it's rather

tuples = db.execute sql
raise "something terrible has happened in the database" unless
tuples.first && tuples.first.first == 42

which doesn't look too bad IMHO.

Regards

robert

 

[Joel VanderWerf]

3)

tuples = db.execute sql

tuple = tuples.first
answer = tuple.first

raise "something terrible has happened in the database" unless
answer == 42

Actually, (3) is even worse that it looks here, because you have to test
tuple before sending #first to it, if you want the same semantics as (1)
and (2).

(3b)
tuples = db.execute sql

tuple = tuples.first
raise "something terrible has happened in the database" unless tuple

answer = tuple.first
raise "something terrible has happened in the database" unless
answer == 42

I guess you could put a "rescue NoMethodError" clause around everything,
but that would hide any NoMethodError that came up in, say, #first. So
the argument for (1) is pretty strong...

OTOH, (3b) has the advantage that you can make the error message more
informative in each of the two error cases.

If someone wants a conditional with a short-circuitable sequence of
tests, but without the compact elegance of (1), they could do something
like this:

def the_following_checks_succeed
catch :fail do yield end
end

def check
yield or throw :fail, false
end

tuples = [ [42,2,3], [4,5,6] ]
raise "something's wrong" unless the_following_checks_succeed do
check {tuples}
tuple = check {tuples[0]}
answer = check {tuple[0]}
answer == 42 # or: check {answer == 42}
end
puts "Now, what is the question?"

 

[Rasputin]

There are not always warnings; it seems, it's only warned if the right side
is a literal (i.e. a string or a number). This is reasonable because it is
likely that a comparison was meant. After all, what do you gain by doing

if ( x = 10 )
end

over

if ( 10 )
end

?

Actually, I don't think(not sure about this though) that Ruby actually
knows, whether the number/string/whatever is in the literal(?) form, or
if it's a return value from a function, beyond the parser level.
Therefore, checking for it should be hard (or better yet, a waste of
cycles).

Personally, I think this warning should be allegated to the
$VERBOSE == true level. (Those would be much easier to ignore)

What I wanted to do was check if an array contained a certain value, and
do something with it, or else. Simple matter.

Just a mite obfusticated, if at all. So I think the stuff above made
sense... (It was short and to the point anyway (i think...))

But this is just going to be a matter that will be discussed my many
without anyone changing their minds (not really they won't).

I mean should we go for short elegant code irritating-waste-of-spacers.
Ha.

 

[Robert Klemme]

Actually, I don't think(not sure about this though) that Ruby actually
knows, whether the number/string/whatever is in the literal(?) form, or
if it's a return value from a function, beyond the parser level.

It's sufficient to know that during parsing, because this is a syntax
warning:

$ ruby -c -e 'if x = 10; puts "ja"; end'
-e:1: warning: found = in conditional, should be ==
Syntax OK

Therefore, checking for it should be hard (or better yet, a waste of
cycles).

With that argument you'd have to do assembler only - every syntax and other
check then is a waste of cycles. Apart from that: the check is only done
once during compilation. There's no runtime overhead at all.

Personally, I think this warning should be allegated to the
$VERBOSE == true level. (Those would be much easier to ignore)

I beg to differ. Rather write code that does not give you warnings. You
should at least consider the option that there is actually a good reason for
these warnings.

What I wanted to do was check if an array contained a certain value, and
do something with it, or else. Simple matter.

How then did you get a warning?

$ ruby -c -e 'a=%w{a b d c}; while a.include? "x"; p x; end'
Syntax OK

(no warning here)

Just a mite obfusticated, if at all. So I think the stuff above made
sense... (It was short and to the point anyway (i think...))

Which stuff? Did you post your code? I can't see it at the moment. Please
show us the code; I bet we can then discuss this better.

But this is just going to be a matter that will be discussed my many
without anyone changing their minds (not really they won't).

Well, I'm open to change my mind if you provide better arguments in favor of
your position. It's just that I can't see them at the moment.

I mean should we go for short elegant code irritating-waste-of-spacers.

I don't know what's irritating about

x = get_x()

if x == "foo"
puts "ja"
end

I find this more irritating:

if ( x = get_x() ) == "foo"
puts "ja"
end

I prefer to have this only with loops because in that case it's a real gain
in elegance that can't be achieved otherwise. But for if/unless it's plain
superfluous.

Regards

robert