Can ik make this more beautifull?

ruby-talk
1

nowadays i try to improve my coding style to produce nicer and beter
readable code. I try to improve the following code but i dont know how.

if condtion1
      x=1
    else
      if condition2
         x=2
      else
         if condition3
           x=3
        end
      end
      x=nill
  end

there are three different conditions which can not occur at the same
time
can i make this nicer?

···

--
Posted via http://www.ruby-forum.com/.

2

Actually, when you set x to nil, in the end of the code, you're losing
your entire second "if" clause. Unless these conditions are calls to
other functions or something like that, you could refactor your code
this way:

x = condition1 ? 1 : nil

Best regards!

Felipe Giotto :wink:

···

On Jan 31, 2008 9:52 AM, Remco Hh <remco@huijdts.nl> wrote:

nowadays i try to improve my coding style to produce nicer and beter
readable code. I try to improve the following code but i dont know how.

if condtion1
      x=1
    else
      if condition2
         x=2
      else
         if condition3
           x=3
        end
      end
      x=nill
  end

there are three different conditions which can not occur at the same
time
can i make this nicer?
--
Posted via http://www.ruby-forum.com/\.

--
Felipe Luiz Christófolli Giotto
Inovare Development Team
http://www.inovare.net

3

http://www.ruby-doc.org/docs/ProgrammingRuby/html/tut_expressions.html#S5

HTH,

Felix

···

On Thu, 2008-01-31 at 20:52 +0900, Remco Hh wrote:

nowadays i try to improve my coding style to produce nicer and beter
readable code. I try to improve the following code but i dont know how.

if condtion1
      x=1
    else
      if condition2
         x=2
      else
         if condition3
           x=3
        end
      end
      x=nill
  end

there are three different conditions which can not occur at the same
time
can i make this nicer?

4

Remco Hh wrote:

nowadays i try to improve my coding style to produce nicer and beter
readable code. I try to improve the following code but i dont know how.

if condtion1
      x=1
    else
      if condition2
         x=2
      else
         if condition3
           x=3
        end
      end
      x=nill
  end

there are three different conditions which can not occur at the same
time
can i make this nicer?

A case expression is probably the most clear alternative. The shortest is perhaps something like:

x = condition1 ? 1 : condition2 ? 2 : 3

Best regards,

Jari Williamsson

5

Jari Williamsson wrote:

Remco Hh wrote:

nowadays i try to improve my coding style to produce nicer and beter
readable code. I try to improve the following code but i dont know how.

if condtion1
      x=1
    else
      if condition2
         x=2
      else
         if condition3
           x=3
        end
      end
      x=nill
  end

there are three different conditions which can not occur at the same
time
can i make this nicer?

A case expression is probably the most clear alternative. The shortest is perhaps something like:

x = condition1 ? 1 : condition2 ? 2 : 3

Best regards,

Jari Williamsson

Short it may be but this sort of code can hardly be called beautiful, I have had to maintain crap like this before and the few keystrokes the author saved was made up for 1000 times when someone updated it and things didn't 'quite' work how they thought. Ruby allows simplicity and clarity, this however is heading into obfuscation.

The better solution is to use the elsif keyword to flatten the indentation:

if (temp == 'cat')
  x = 1
elsif(temp == 'dog')
  x = 2
elsif(temp == 'banana')
  x = 3
else
  x = nil
end

6

Felipe Giotto ha scritto:

Actually, when you set x to nil, in the end of the code, you're losing
your entire second "if" clause. Unless these conditions are calls to
other functions or something like that, you could refactor your code
this way:

x = condition1 ? 1 : nil

Best regards!

Felipe Giotto :wink:

nowadays i try to improve my coding style to produce nicer and beter
readable code. I try to improve the following code but i dont know how.

if condtion1
      x=1
    else
      if condition2
         x=2
      else
         if condition3
           x=3
        end
      end
      x=nill
  end

there are three different conditions which can not occur at the same
time
can i make this nicer?

just learn to quote... http://www.carnackyweb.com/?p=30

···

On Jan 31, 2008 9:52 AM, Remco Hh <remco@huijdts.nl> wrote:

7

Which is analogous to the case statement:

case temp
when 'cat': 1
when 'dog': 2
when 'banana': 3
else nil
end

···

On Jan 31, 8:47 am, Peter Hickman <pe...@semantico.com> wrote:

Jari Williamsson wrote:
> Remco Hh wrote:
>> nowadays i try to improve my coding style to produce nicer and beter
>> readable code. I try to improve the following code but i dont know how.

>> if condtion1
>> x=1
>> else
>> if condition2
>> x=2
>> else
>> if condition3
>> x=3
>> end
>> end
>> x=nill
>> end

>> there are three different conditions which can not occur at the same
>> time
>> can i make this nicer?

> A case expression is probably the most clear alternative. The shortest
> is perhaps something like:

> x = condition1 ? 1 : condition2 ? 2 : 3

> Best regards,

> Jari Williamsson

Short it may be but this sort of code can hardly be called beautiful, I
have had to maintain crap like this before and the few keystrokes the
author saved was made up for 1000 times when someone updated it and
things didn't 'quite' work how they thought. Ruby allows simplicity and
clarity, this however is heading into obfuscation.

The better solution is to use the elsif keyword to flatten the indentation:

if (temp == 'cat')
  x = 1
elsif(temp == 'dog')
  x = 2
elsif(temp == 'banana')
  x = 3
else
  x = nil
end

8

Peter Hickman wrote:

Jari Williamsson wrote:

Remco Hh wrote:

nowadays i try to improve my coding style to produce nicer and beter
readable code. I try to improve the following code but i dont know how.

if condtion1
      x=1
    else
      if condition2
         x=2
      else
         if condition3
           x=3
        end
      end
      x=nill
  end

there are three different conditions which can not occur at the same
time
can i make this nicer?

A case expression is probably the most clear alternative. The shortest is perhaps something like:

x = condition1 ? 1 : condition2 ? 2 : 3

Best regards,

Jari Williamsson

Short it may be but this sort of code can hardly be called beautiful, I have had to maintain crap like this before and the few keystrokes the author saved was made up for 1000 times when someone updated it and things didn't 'quite' work how they thought. Ruby allows simplicity and clarity, this however is heading into obfuscation.

The better solution is to use the elsif keyword to flatten the indentation:

if (temp == 'cat')
x = 1
elsif(temp == 'dog')
x = 2
elsif(temp == 'banana')
x = 3
else
x = nil
end

But this elsif solution isn't heading for clarity either, compared to using a case expression:
x = case temp
         when 1 then 'cat'
         when 2 then 'dog'
         when 3 then 'banana'
         else nil
     end

In the elsif example, x appears 4 times instead of 1, temp appears 3 times instead of 1. Apart from require an editor that support refactoring and the risk of typos, there will also be a performance hit when temp isn't matched.

Best regards,

Jari Williamsson

9

That is apparently a matter of opinion.
I find the single line alternative clear, concise and quite beautiful.
It reads just like a sentence for me.

The 9-line alternative is annoyingly verbose and just burns vertical
space.
That style has me paging up and down or reaching for the scroll bar more
often.
Having to do that does not make the code easier to read in my opinion.

-- Mike Berrow

···

---------------------------
Peter Hickman wrote:

Jari Williamsson wrote:

         if condition3

A case expression is probably the most clear alternative. The shortest
is perhaps something like:

x = condition1 ? 1 : condition2 ? 2 : 3

Best regards,

Jari Williamsson

Short it may be but this sort of code can hardly be called beautiful, I
have had to maintain crap like this before and the few keystrokes the
author saved was made up for 1000 times when someone updated it and
things didn't 'quite' work how they thought. Ruby allows simplicity and
clarity, this however is heading into obfuscation.

The better solution is to use the elsif keyword to flatten the
indentation:

if (temp == 'cat')
  x = 1
elsif(temp == 'dog')
  x = 2
elsif(temp == 'banana')
  x = 3
else
  x = nil
end

--
Posted via http://www.ruby-forum.com/\.

10

Hi,

Jari Williamsson wrote:
if (temp == 'cat')
x = 1
elsif(temp == 'dog')
x = 2
elsif(temp == 'banana')
x = 3
else
x = nil
end

  x = if cond1 then
    1
  elsif cond2 then
    2
  elsif cond3 then
    3
  end

The else clause is superfluous.

Bertram

···

Am Donnerstag, 31. Jan 2008, 22:47:22 +0900 schrieb Peter Hickman:

--
Bertram Scharpf
Stuttgart, Deutschland/Germany
http://www.bertram-scharpf.de

11

Since your making a choice, a hash seems like it might be a tidy
option:

  answers = {condition1=>1, condition2=>2, condition3=>3}
  x = answers[condition]

As for the oneline version, I always find it particulary confusing in
Ruby as you can have ? at the end of a method or it can be used in
front a character to get the characters ASCII code. And nesting just
multiplies the problemn, IMHO

cheers

···

On Jan 31, 9:14 am, Mike Berrow <mberr...@pacbell.net> wrote:

That is apparently a matter of opinion.
I find the single line alternative clear, concise and quite beautiful.
It reads just like a sentence for me.

The 9-line alternative is annoyingly verbose and just burns vertical
space.
That style has me paging up and down or reaching for the scroll bar more
often.
Having to do that does not make the code easier to read in my opinion.

-- Mike Berrow

---------------------------

Peter Hickman wrote:
> Jari Williamsson wrote:
>>> if condition3
>> A case expression is probably the most clear alternative. The shortest
>> is perhaps something like:

>> x = condition1 ? 1 : condition2 ? 2 : 3

>> Best regards,

>> Jari Williamsson

> Short it may be but this sort of code can hardly be called beautiful, I
> have had to maintain crap like this before and the few keystrokes the
> author saved was made up for 1000 times when someone updated it and
> things didn't 'quite' work how they thought. Ruby allows simplicity and
> clarity, this however is heading into obfuscation.

> The better solution is to use the elsif keyword to flatten the
> indentation:

> if (temp == 'cat')
> x = 1
> elsif(temp == 'dog')
> x = 2
> elsif(temp == 'banana')
> x = 3
> else
> x = nil
> end

--
Posted viahttp://www.ruby-forum.com/.

12

If you are using "case" then there is yet another solution:

x = case
  when condition1 then 1
  when condition2 then 2
  when condition3 then 3
  else nil
end

Another approach would be to stuff conditions in an array:

CONDITIONS = [
  lambda {|x| condition1 },
  lambda {|x| condition2 },
  lambda {|x| condition3 },
]

def test(*values)
  CONDITIONS.each_with_index do |c,i|
    return i+1 if c[*values]
  end
  nil
end

x = test "whatever"

Kind regards

robert

···

2008/1/31, Jari Williamsson <jari.williamsson@mailbox.swipnet.se>:

Peter Hickman wrote:
> Jari Williamsson wrote:
>> Remco Hh wrote:
>>> nowadays i try to improve my coding style to produce nicer and beter
>>> readable code. I try to improve the following code but i dont know how.
>>>
>>> if condtion1
>>> x=1
>>> else
>>> if condition2
>>> x=2
>>> else
>>> if condition3
>>> x=3
>>> end
>>> end
>>> x=nill
>>> end
>>>
>>> there are three different conditions which can not occur at the same
>>> time
>>> can i make this nicer?
>>
>> A case expression is probably the most clear alternative. The shortest
>> is perhaps something like:
>>
>> x = condition1 ? 1 : condition2 ? 2 : 3
>>
>>
>> Best regards,
>>
>> Jari Williamsson
>>
>>
> Short it may be but this sort of code can hardly be called beautiful, I
> have had to maintain crap like this before and the few keystrokes the
> author saved was made up for 1000 times when someone updated it and
> things didn't 'quite' work how they thought. Ruby allows simplicity and
> clarity, this however is heading into obfuscation.
>
> The better solution is to use the elsif keyword to flatten the indentation:
>
> if (temp == 'cat')
> x = 1
> elsif(temp == 'dog')
> x = 2
> elsif(temp == 'banana')
> x = 3
> else
> x = nil
> end

But this elsif solution isn't heading for clarity either, compared to
using a case expression:
x = case temp
         when 1 then 'cat'
         when 2 then 'dog'
         when 3 then 'banana'
         else nil
     end

In the elsif example, x appears 4 times instead of 1, temp appears 3
times instead of 1. Apart from require an editor that support
refactoring and the risk of typos, there will also be a performance hit
when temp isn't matched.

--
use.inject do |as, often| as.you_can - without end

13

Jari Williamsson wrote:

But this elsif solution isn't heading for clarity either, compared to using a case expression:
x = case temp
        when 1 then 'cat'
        when 2 then 'dog'
        when 3 then 'banana'
        else nil
    end

In the elsif example, x appears 4 times instead of 1, temp appears 3 times instead of 1. Apart from require an editor that support refactoring and the risk of typos, there will also be a performance hit when temp isn't matched.

Indeed this is a nice solution to the problem of assigning a value. However the 'temp == ...' was of my own invention and not part of the OP. That simply gave 'condition1', 'condition2' etc so there is a strong possibility that what you propose would not work for him as the conditions might not be based upon the same variable as in my example, if the OP is new to programming you will have led him down the wrong path by not pointing out that your solution only works for conditions based upon the conditions all being tests of temp.

Also when posting code it helps to run it, no matter how much of a genius you are. Your code does not do what mine did. Try this instead:

x = case temp
        when 'cat' then 1
        when 'dog' then 2
        when 'banana' then 3
        else nil
        end

I would worry less about how many keystrokes you have saved and spend a little more time getting it right and testing your code. You wrote 3 lines less than me and got it completely backwards.

14

Mike Berrow wrote:

That is apparently a matter of opinion.
I find the single line alternative clear, concise and quite beautiful.
It reads just like a sentence for me.

The 9-line alternative is annoyingly verbose and just burns vertical space.
That style has me paging up and down or reaching for the scroll bar more often.
Having to do that does not make the code easier to read in my opinion.
  
When you have to debug or maintain multiply nested ?: logic you will find yourself questioning the parentage of the original programmer. ?: is useful to keep minor details from getting verbose, nested ?: is the path to madness.

15

Mike Berrow ha scritto:

That is apparently a matter of opinion.
I find the single line alternative clear, concise and quite beautiful.
It reads just like a sentence for me.

The 9-line alternative is annoyingly verbose and just burns vertical space.
That style has me paging up and down or reaching for the scroll bar more often.
Having to do that does not make the code easier to read in my opinion.

-- Mike Berrow

---------------------------
Peter Hickman wrote:

Jari Williamsson wrote:

         if condition3

A case expression is probably the most clear alternative. The shortest is perhaps something like:

x = condition1 ? 1 : condition2 ? 2 : 3

Best regards,

Jari Williamsson

Short it may be but this sort of code can hardly be called beautiful, I
have had to maintain crap like this before and the few keystrokes the
author saved was made up for 1000 times when someone updated it and
things didn't 'quite' work how they thought. Ruby allows simplicity and
clarity, this however is heading into obfuscation.

The better solution is to use the elsif keyword to flatten the indentation:

if (temp == 'cat')
  x = 1
elsif(temp == 'dog')
  x = 2
elsif(temp == 'banana')
  x = 3
else
  x = nil
end

quoting...this stranger!! http://www.carnackyweb.com/?p=30

16

On a more general note, here are some guidelines I use to write clear
code. All of these guidelines follow the principle of lightening the
load on your working memory:

(1) Name variables what you call them. If you have a variable named
a and someone asks, "What's a?" and you say "That's the amount of time
left", then rename a to time_left.
(2) In general, avoid abbreviations. The time you save typing is
wasted in other areas. While it may be completely obvious to you that
"prev" means preview, the comment next to the code that mentions
"previous" values can confuse the issue (yes, this is a real-life
example). Abbreviations also may force the user of your code to have
to look up method names more often (Now was that method called prev or
previous?)
(3) Write cohesively. Ideally, each part of the system should be
responsible for one clearly-expressed thing. Each method should have
one purpose, each class should have one overarching, easily-expressed
purpose, etc.

···

On Jan 31, 10:18 am, Peter Hickman <pe...@semantico.com> wrote:

Jari Williamsson wrote:
> But this elsif solution isn't heading for clarity either, compared to
> using a case expression:
> x = case temp
> when 1 then 'cat'
> when 2 then 'dog'
> when 3 then 'banana'
> else nil
> end

> In the elsif example, x appears 4 times instead of 1, temp appears 3
> times instead of 1. Apart from require an editor that support
> refactoring and the risk of typos, there will also be a performance
> hit when temp isn't matched.

Indeed this is a nice solution to the problem of assigning a value.
However the 'temp == ...' was of my own invention and not part of the
OP. That simply gave 'condition1', 'condition2' etc so there is a strong
possibility that what you propose would not work for him as the
conditions might not be based upon the same variable as in my example,
if the OP is new to programming you will have led him down the wrong
path by not pointing out that your solution only works for conditions
based upon the conditions all being tests of temp.

Also when posting code it helps to run it, no matter how much of a
genius you are. Your code does not do what mine did. Try this instead:

x = case temp
        when 'cat' then 1
        when 'dog' then 2
        when 'banana' then 3
        else nil
        end

I would worry less about how many keystrokes you have saved and spend a
little more time getting it right and testing your code. You wrote 3
lines less than me and got it completely backwards.

17

Peter Hickman wrote:

Mike Berrow wrote:

That is apparently a matter of opinion.
I find the single line alternative clear, concise and quite beautiful.
It reads just like a sentence for me.
..

When you have to debug or maintain multiply nested ?: logic you will
find yourself questioning the parentage of the original programmer. ?:
is useful to keep minor details from getting verbose, nested ?: is the
path to madness.

Well you might, but I won't. Like I said, it's a matter of opinion.
I don't have problem with nested ?:
I usually use brackets with them. For me that's works as well
as indentation and verticality.

-- Mike

···

--
Posted via http://www.ruby-forum.com/\.