Help me reduce my script size

Ruby
1

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?

2

On Mon, Sep 16, 2013 at 5:36 PM, felix chang [email protected]
wrote:

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?

Untested:

Infinity = 1.0 / 0

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.

3

On Mon, Sep 16, 2013 at 11:36 AM, felix chang [email protected]
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

4

On Sep 16, 2013, at 11:06 AM, Dave A.
[email protected] wrote:

something constant do |r|
File.open(file,“r”).each_slice(4) do |r|
that as an exercise for the reader.) Then call that where it says “we
should increment”.

-Dave

–
Dave A., 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

5

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 A. wrote in post #1121565:

On Mon, Sep 16, 2013 at 11:36 AM, felix chang [email protected]
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

6

On Tue, Sep 17, 2013 at 3:42 AM, Dave A.
[email protected] wrote:

On Mon, Sep 16, 2013 at 9:03 PM, felix chang [email protected] wrote:

File.open(file,“r”).each_slice(4) do |_id,seq,*|
counter.call seq.chomp, min, max
end
end

Since nobody seems to have pointed it out: the file descriptor is not
properly closed. Better do

File.open file do |io|
io.each_slice 4 do |_id, seq,*|
…
end
end

or

File.foreach(file).each_slice 4 do |_id, seq,*|
…
end

Cheers

robert

7

On Mon, Sep 16, 2013 at 6:03 PM, felix chang [email protected]
wrote:

elsif min.nil? && max
end

Why not this?

def self.count(file, min, max)

def self.count(file, min, max)
tags = Hash.new(0)

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

8

Why not this?

Because I was wrong

Let’s try this instead

def self.count(file, min, max)

def self.count(file, min, max)
tags = Hash.new(0)

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

I read it completely wrong before. Sorry

John

9

On Mon, Sep 16, 2013 at 9:03 PM, felix chang [email protected]
wrote:

File.open(file,“r”).each_slice(4) do |_id,seq,*|
counter.call seq.chomp, min, max
end
end

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