def self.count file,min,max
tags = Hash.new(0)
if min && max
File.open(file,"r").each_slice(4) do |r|
seq = r[1].chomp
tags[seq] += 1 if seq.length.between? min,max
end
elsif min.nil? && max
File.open(file,"r").each_slice(4) do |r|
seq = r[1].chomp
tags[seq] += 1 if seq.length <= max
end
elsif min && max.nil?
File.open(file,"r").each_slice(4) do |r|
seq = r[1].chomp
tags[seq] += 1 if seq.length >= min
end
else
File.open(file,"r").each_slice(4) do |r|
seq = r[1].chomp
tags[seq] += 1
end
end
end
Yes! you can see, my code is redundant. But I dont know how to reduce
it.
Does any body have some suggestion?
Yes. First, take a look at the pattern in which your code varies.
Each piece is of the form:
if some varying condition depending on min and max
something constant do |r|
do another constant thing
do some other constant thing if another varying condition of
min, max, and a block-var
end
end
Since you have an "else" block that is likewise of this form, all
possibilities are covered. So, let's see if we can take this out of
the if. Then we have:
File.open(file,"r").each_slice(4) do |r|
seq = r[1].chomp
tags[seq] += 1 if we should increment
end
and we just need to define "we should increment". We could put a
logical expression there, that looks at the various things needed to
make the decision (looks at the first condition and applies the
appropriate second condition), but that could get rather hairy. So
what I would advise in this case is to extract that to a method that
accepts those parameters, and gives back a true or false. (I'll leave
that as an exercise for the reader.) Then call that where it says "we
should increment".
-Dave
···
On Mon, Sep 16, 2013 at 11:36 AM, felix chang <lists@ruby-forum.com> wrote:
Yes! you can see, my code is redundant. But I dont know how to reduce
it.
Does any body have some suggestion?
--
Dave Aronson, the T. Rex of Codosaurus LLC,
secret-cleared freelance software developer
taking contracts in or near NoVa or remote.
See information at http://www.Codosaur.us/\.
Sometimes I wonder if learning how and what to refactor is like learning how to solve quadratic equations. Then I remember that I no longer remember how to do that, and go back to refactoring…
···
On Sep 16, 2013, at 11:06 AM, Dave Aronson <rubytalk2dave@davearonson.com> wrote:
On Mon, Sep 16, 2013 at 11:36 AM, felix chang <lists@ruby-forum.com> wrote:
Yes! you can see, my code is redundant. But I dont know how to reduce
it.
Does any body have some suggestion?
Yes. First, take a look at the pattern in which your code varies.
Each piece is of the form:
if some varying condition depending on min and max
something constant do |r|
do another constant thing
do some other constant thing if another varying condition of
min, max, and a block-var
end
end
Since you have an "else" block that is likewise of this form, all
possibilities are covered. So, let's see if we can take this out of
the if. Then we have:
File.open(file,"r").each_slice(4) do |r|
seq = r[1].chomp
tags[seq] += 1 if we should increment
end
and we just need to define "we should increment". We could put a
logical expression there, that looks at the various things needed to
make the decision (looks at the first condition and applies the
appropriate second condition), but that could get rather hairy. So
what I would advise in this case is to extract that to a method that
accepts those parameters, and gives back a true or false. (I'll leave
that as an exercise for the reader.) Then call that where it says "we
should increment".
-Dave
--
Dave Aronson, the T. Rex of Codosaurus LLC,
secret-cleared freelance software developer
taking contracts in or near NoVa or remote.
See information at http://www.Codosaur.us/\.
def self.count file,min=-Infinity,max=Infinity
tags = Hash.new(0)
File.open(file,"r").each_slice(4) do |r|
seq = r[1].chomp
tags[seq] += 1 if seq.length.between? min,max
end
end
Hope this helps,
Jesus.
···
On Mon, Sep 16, 2013 at 5:36 PM, felix chang <lists@ruby-forum.com> wrote:
Here is my code:
def self.count file,min,max
tags = Hash.new(0)
if min && max
File.open(file,"r").each_slice(4) do |r|
seq = r[1].chomp
tags[seq] += 1 if seq.length.between? min,max
end
elsif min.nil? && max
File.open(file,"r").each_slice(4) do |r|
seq = r[1].chomp
tags[seq] += 1 if seq.length <= max
end
elsif min && max.nil?
File.open(file,"r").each_slice(4) do |r|
seq = r[1].chomp
tags[seq] += 1 if seq.length >= min
end
else
File.open(file,"r").each_slice(4) do |r|
seq = r[1].chomp
tags[seq] += 1
end
end
end
Yes! you can see, my code is redundant. But I dont know how to reduce
it.
Does any body have some suggestion?
Thanks for your suggestion. I have try to refactor my code as list
below:
def self.count file,min,max
tags = Hash.new(0)
if min && max
counter = lambda {|seq,mi,ma| tags[seq] += 1 if seq.size.between?
mi,ma }
elsif min.nil? && max
counter = lambda {|seq,mi,ma| tags[seq] += 1 if seq.size <= ma }
elsif max.nil? && min
counter = lambda {|seq,mi,ma| tags[seq] += 1 if seq.size >= mi }
else
counter = lambda {|seq,mi,ma| tags[seq] += 1 }
end
File.open(file,"r").each_slice(4) do |_id,seq,*|
counter.call seq.chomp, min, max
end
end
Dave Aronson wrote in post #1121565:
···
On Mon, Sep 16, 2013 at 11:36 AM, felix chang <lists@ruby-forum.com> > wrote:
Yes! you can see, my code is redundant. But I dont know how to reduce
it.
Does any body have some suggestion?
Yes. First, take a look at the pattern in which your code varies.
Each piece is of the form:
if some varying condition depending on min and max
something constant do |r|
do another constant thing
do some other constant thing if another varying condition of
min, max, and a block-var
end
end
Since you have an "else" block that is likewise of this form, all
possibilities are covered. So, let's see if we can take this out of
the if. Then we have:
File.open(file,"r").each_slice(4) do |r|
seq = r[1].chomp
tags[seq] += 1 if we should increment
end
and we just need to define "we should increment". We could put a
logical expression there, that looks at the various things needed to
make the decision (looks at the first condition and applies the
appropriate second condition), but that could get rather hairy. So
what I would advise in this case is to extract that to a method that
accepts those parameters, and gives back a true or false. (I'll leave
that as an exercise for the reader.) Then call that where it says "we
should increment".
Interesting! I was thinking somewhat along those lines as an
intermediate step, but thought that someone having trouble with
refactoring might not know enough Ruby to do lambdas quite yet, and
that we might as well skip straight to a more final form using a
function like this:
def we_should_increment(seq, min, max)
return true unless min || max
len = seq.size
return len.between?(min, max) if min && max
return len <= max if max
len >= min
end
Anyway, back to your "pick a lambda" solution, your lambdas are still
a bit wet. There are a couple more things you can do to DRY it up, if
you really want....
-Dave
···
On Mon, Sep 16, 2013 at 9:03 PM, felix chang <lists@ruby-forum.com> wrote:
if min && max
counter = lambda {|seq,mi,ma| tags[seq] += 1 if seq.size.between?
mi,ma }
elsif min.nil? && max
counter = lambda {|seq,mi,ma| tags[seq] += 1 if seq.size <= ma }
elsif max.nil? && min
counter = lambda {|seq,mi,ma| tags[seq] += 1 if seq.size >= mi }
else
counter = lambda {|seq,mi,ma| tags[seq] += 1 }
end
File.open(file,"r").each_slice(4) do |_id,seq,*|
counter.call seq.chomp, min, max
end
end
--
Dave Aronson, the T. Rex of Codosaurus LLC,
secret-cleared freelance software developer
taking contracts in or near NoVa or remote.
See information at http://www.Codosaur.us/\.
File.open(file) do |io|
io.each_slice 4 do |_id, seq, fmin, fmax|
work_min = min || fmin
work_max = max || fmax
tags[seq] += 1 if seq.size >= work_min && seq.size <= work_max
end
end
end
I'm assuming mi and ma come from the next two members of the slice but it
actually is a little hazy as you have it written.
John
···
On Mon, Sep 16, 2013 at 6:03 PM, felix chang <lists@ruby-forum.com> wrote:
Thanks for your suggestion. I have try to refactor my code as list
below:
def self.count file,min,max
tags = Hash.new(0)
if min && max
counter = lambda {|seq,mi,ma| tags[seq] += 1 if seq.size.between?
mi,ma }
elsif min.nil? && max
counter = lambda {|seq,mi,ma| tags[seq] += 1 if seq.size <= ma }
elsif max.nil? && min
counter = lambda {|seq,mi,ma| tags[seq] += 1 if seq.size >= mi }
else
counter = lambda {|seq,mi,ma| tags[seq] += 1 }
end
File.open(file,"r").each_slice(4) do |_id,seq,*|
counter.call seq.chomp, min, max
end
end
File.open(file) do |io|
io.each_slice 4 do |_id, seq, *|
seq = seq.chomp
work_min = min || seq.size
work_max = max || seq.size
tags[seq] += 1 if seq.size >= work_min && seq.size <= work_max
end
end
end