> def test
> while 1
> puts (1..9).each_slice(3).map do |slice|
> sum = slice.inject do |acc, ele|
> acc + ele
> end
>
> if sum > 6
> 42
> else
> 99
> end~if
> end~do.join ','
> end~while
> end
>
Hi,
I know it's a bit of a contrived example, and I'm not advocating a version
of "Ruby Golf" here, but if I saw this method in some code I was maintaining
I would refactor it to something like:
def test
while 1
puts (1..9).each_slice(3).map do |slice|
sum = slice.inject { |acc, ele| acc + ele }
sum > 6 ? 42 : 99
end.join(',')
end
end
...or even better, move the "summing" to another method...
def test
while 1
puts (1..9).each_slice(3).map do |slice|
calculate_sum > 6 ? 42 : 99
end.join(',')
end
end
def calculate_sum(items)
items.inject { |acc, ele| acc + ele }
end
I think most cases where the end~if syntax is "necessary" would be better off
(e.g. more readable) with a healthy dose of refactoring.
Of course, the example was made up to demonstrate the ends and not real
world code. I would not write it in this way, either...
This one is from existing code:
module ABC
# ...
def abc
x.each do
if x
case lorem
when :ipsum
# ...
when :ipsum
# ...
when :ipsum
# ...
when :ipsum
# ...
else
# ...
end
end
end
end
end
My general rule of thumb is to put long case statements (if I can't avoid
using them) into their own separate method. So I would refactor the code to:
module ABC
def abc
x.each do
some_method_name_that_describes_what_case_statement_does(lorem) if x
end
end
def some_method_name_that_describes_what_case_statement_does(value)
case lorem
when :ipsum
# ...
when :ipsum
# ...
when :ipsum
# ...
when :ipsum
# ...
else
# ...
end
end
end
As an aside, take a look at Refactoring: Ruby Edition[1]. It covers ways of
dealing with long methods, case statements, and other situations that
cause difficulties trying
to match up "end"'s.
On Mon, Jul 12, 2010 at 3:30 PM, Jan Lelis <prog@janlelis.de> wrote:
> def test
> while 1
> puts (1..9).each_slice(3).map do |slice|
> sum = slice.inject do |acc, ele|
> acc + ele
> end
>
> if sum > 6
> 42
> else
> 99
> end~if
> end~do.join ','
> end~while
> end
>
Hi,
I know it's a bit of a contrived example, and I'm not advocating a version
of "Ruby Golf" here, but if I saw this method in some code I was maintaining
I would refactor it to something like:
def test
while 1
puts (1..9).each_slice(3).map do |slice|
sum = slice.inject { |acc, ele| acc + ele }
sum > 6 ? 42 : 99
end.join(',')
end
end
...or even better, move the "summing" to another method...
def test
while 1
puts (1..9).each_slice(3).map do |slice|
calculate_sum > 6 ? 42 : 99
end.join(',')
end
end
def calculate_sum(items)
items.inject { |acc, ele| acc + ele }
end
I think most cases where the end~if syntax is "necessary" would be better off
(e.g. more readable) with a healthy dose of refactoring.
Of course, the example was made up to demonstrate the ends and not real
world code. I would not write it in this way, either...
This one is from existing code:
module ABC
# ...
def abc
x.each do
if x
case lorem
when :ipsum
\# \.\.\.
when :ipsum
\# \.\.\.
when :ipsum
\# \.\.\.
when :ipsum
\# \.\.\.
else
\# \.\.\.
end
end
end
> def test
> while 1
> puts (1..9).each_slice(3).map do |slice|
> sum = slice.inject do |acc, ele|
> acc + ele
> end
>
> if sum > 6
> 42
> else
> 99
> end~if
> end~do.join ','
> end~while
> end
>
Hi,
I know it's a bit of a contrived example, and I'm not advocating a version
of "Ruby Golf" here, but if I saw this method in some code I was maintaining
I would refactor it to something like:
def test
while 1
puts (1..9).each_slice(3).map do |slice|
sum = slice.inject { |acc, ele| acc + ele }
sum > 6 ? 42 : 99
end.join(',')
end
end
...or even better, move the "summing" to another method...
def test
while 1
puts (1..9).each_slice(3).map do |slice|
calculate_sum > 6 ? 42 : 99
end.join(',')
end
end
def calculate_sum(items)
items.inject { |acc, ele| acc + ele }
end
I think most cases where the end~if syntax is "necessary" would be better off
(e.g. more readable) with a healthy dose of refactoring.
Of course, the example was made up to demonstrate the ends and not real
world code. I would not write it in this way, either...
This one is from existing code:
module ABC
# ...
def abc
x.each do
if x
This is probably a different criterion, otherwise the "if" statement
would be superfluous.
case lorem
when :ipsum
\# \.\.\.
when :ipsum
\# \.\.\.
when :ipsum
\# \.\.\.
when :ipsum
\# \.\.\.
else
\# \.\.\.
end
end
end
end
end
If you have lengthy case statements you should consider a different
approach because they are relatively slow and awkward to read (as you
noticed already). State and strategy pattern come to mind. Often it
is also advisable to do something like this:
def work(obj)
MY_OPERATION.fetch(obj.class) do |cl|
raise ArgumentError, "Dunno what to do with #{obj.inspect}"
end[obj]
end
end
irb(main):014:0> f = Foo.new
=> #<Foo:0x10297f34>
irb(main):015:0> f.work 123
=> 123
irb(main):016:0> f.work "456"
=> 456
irb(main):017:0> f.work $stdin
789
=> 789
irb(main):018:0> f.work [1,2,3]
ArgumentError: Dunno what to do with [1, 2, 3]
from (irb):10:in `block in work'
from (irb):9:in `fetch'
from (irb):9:in `work'
from (irb):18
from /opt/bin/irb19:12:in `<main>'
irb(main):019:0>
You can view this as a way to add methods to types without modifying them.
In favour of:
*) Keeping with designer's intentions. Least surprise being one of.
*) Keeping understandability as a premium.
*) Using inline documentation where self.doco is not enough.
*) Readability is key, using less verbosity being part of this.
*) Refactoring can be from different perspectives, one's own code, an
imported library or maintenance.
Refactoring a previous post I would use hanging indents to 'indicate'
methods, compress trailing ends on lastline.
Also, I use one space for each indent, this gives me an indicator at
the end of a .code-block. as to having correct closing(s).
I like to read the code, not the end markers.
Different approaches for different code.
module ABC
def abc
x.each do
some_method_name_that_describes_what_case_statement_does(lorem)
if x end end
def some_method_name_that_describes_what_case_statement_does(value)
case lorem
when :ipsum
# ...
when :ipsum
# ...
when :ipsum
# ...
when :ipsum
# ...
else
# ... end end end