2

Can we make the following Ruby code shorter, and at the same time more readable?

height = do_some_calc
height = 128 if height == 0
Eric Leschinski
  • 146,994
  • 96
  • 417
  • 335
mlzboy
  • 14,343
  • 23
  • 76
  • 97
  • I don't agree with the edites question title, but I don't know better variant, than the previous (in style of "how to write it better/shorter?"). – Nakilon Nov 05 '10 at 17:01
  • @Nakilon I agree with you, but as you said, the previous question title wasn't very clear. – theIV Nov 05 '10 at 20:50

6 Answers6

7
height = 128 if 0 == height = do_some_calc

It's the only way I know, if do_some_calc must be evaluated only once.

Nakilon
  • 34,866
  • 14
  • 107
  • 142
  • Pretty awesome though everything else but elegant, +1 – hurikhan77 Nov 05 '10 at 05:07
  • And it does not pollute the local namespace with a helper variable... :-) (most other solutions do) – hurikhan77 Nov 05 '10 at 05:12
  • In favor of readability I would rewrite this as `height = 128 if (height = do_some_calc) == 0`. – Max Aug 30 '13 at 17:47
  • @Max, if you write in Ruby, then write in Ruby, not C. – Nakilon Aug 30 '13 at 20:08
  • I've been using Ruby for years and it took me a moment to see how `0 == height = do_some_calc` should be parsed. I'll pay the price of two parentheses for a little less ambiguity. Also, what does this have to do with C? – Max Aug 30 '13 at 20:24
2

If you're willing to alter do_some_calc to return false or nil instead of 0, then you're in business. 

height = do_some_calc || 128

If you can't alter do_some_calc to return false or nil when it would normally return 0, then you could wrap it, but you're not saving many characters in the long run. With the exception of the case where you have many places where you set defaults.

This wrapper would return false if do_some_calc returned 0 and the output of do_some_calc in all other cases.

def my_do_some_calc
   temp = do_some_calc 
   temp != 0 && temp
end

Putting it all together gives:

height = my_do_some_calc || 128
EmFi
  • 23,435
  • 3
  • 57
  • 68
1

Shorter? Not and be functional.

  height = (h = do_some_calc).zero? ? 128 : h

as in:

def do_some_calc
  rand 100
end

10.times do 
  height = (h = do_some_calc).zero? ? 128 : h 
  puts height
end
# >> 3
# >> 95
# >> 89
# >> 82
# >> 31
# >> 4
# >> 82
# >> 99
# >> 11
# >> 64
the Tin Man
  • 158,662
  • 42
  • 215
  • 303
0

Maybe something along the lines of:

STANDARD_HEIGHT = 128
def do_some_calc
  height = 0
  #calculate our height...
  height = height == 0 ? STANDARD_HEIGHT : calculated_height
end

I think there needs to be more context given to the 128, hence the constant. I also think that do_some_calc should hide away the fact that if it does equal 0, it really should equal our DEFAULT_HEIGHT.

EDIT: To answer your implied question (that I edited as such), we can make it shorter by making do_some_calc longer.

theIV
  • 25,434
  • 5
  • 54
  • 58
0

You can do as follows

height = do_some_calc.zero? ? 128 : do_some_calc
khanh
  • 4,516
  • 10
  • 29
  • 48
  • I believe the point was to avoid duplicate calls to `do_some_calc`, which might potentially be expensive. – user229044 Nov 05 '10 at 04:34
  • 1
    Well, that was my programming preference. Not specifically stated by OP.... but I would have to believe it to be true. – beach Nov 05 '10 at 04:41
-4

This is technically shorter if not very readable:

(height = do_some_call) == 0 and height = 128

I would say keep it the way you have it, your way seems to be the most concise and readable.

user229044
  • 232,980
  • 40
  • 330
  • 338