what about allowing to specify, which end belongs to whichstart?

Prev: swapping regex libraries
Next: what about allowing to specify, which end belongs to which start?

From: Jan Lelis on 12 Jul 2010 15:30

---

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

From: Bob Nadler on 12 Jul 2010 16:17

---

On Mon, Jul 12, 2010 at 3:30 PM, Jan Lelis <prog(a)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
>    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.

[1] http://www.amazon.com/Refactoring-Ruby-Jay-Fields/dp/0321603508

From: Mark T on 12 Jul 2010 21:33

---

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
---------------------------------------
MarkT

From: Robert Klemme on 13 Jul 2010 04:41

---

2010/7/12 Jan Lelis <prog(a)janlelis.de>:
>> > 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:

class Foo
MY_OPERATION = {
String => lambda {|x| Integer(x)},
Fixnum => lambda {|x| x},
IO => lambda {|x| Integer(x.gets.chomp)}
}

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.

Kind regards

robert

--
remember.guy do |as, often| as.you_can - without end
http://blog.rubybestpractices.com/

First  |  Prev  | 
Pages: 1 2 3 4
Prev: swapping regex libraries
Next: what about allowing to specify, which end belongs to which start?