Refactoring request

Ruby
1

Hi,

I’ve stalled out on making this simpler, and I’m
seeking help pairing this beast down…

Given unused is a hash of arrays where each array has
repeated elements, make elements unique in each array and
remove all names that are not repeated MagicNumber times.
Finally, remove any hash pairs that hold an empty array.

unused.each do |source,routines|
orphans = routines.uniq
orphans.delete_if do |orphan|
repeated = routines.find_all{ |routine| routine == orphan }.size
repeated != MagicNumber
end
unused[source] = orphans
end
unused.delete_if{ |h,k| k.empty? }

Thanks,

2

On Jan 12, 2008, at 9:24 AM, Bil K. wrote:

end
unused[source] = orphans
end
unused.delete_if{ |h,k| k.empty? }

unused = unused.inject(Hash.new) |result, (source, routines)|
orphans = routines.uniq.reject do |o|
routines.inject(0) { |c, r| c + (r == o ? 1 : 0) } != MagicNumber
end
orphans.empty? ? result : result.merge(source => orphans)
end

Hmm, I’m not really sure I did much better there.

James Edward G. II

3

On Jan 12, 2008 4:24 PM, Bil K. [email protected] wrote:

unused.each do |source,routines|
orphans = routines.uniq
orphans.delete_if do |orphan|
repeated = routines.find_all{ |routine| routine == orphan }.size
repeated != MagicNumber
end
unused[source] = orphans
end
unused.delete_if{ |h,k| k.empty? }
Maybe like this

unused.delete_if{ |source, routines|
  routines.delete_if do |o|
    routines.inject(0){ |c,r| c + (r == o ? 1 : 0 ) } != MagicNumber
  end
  routines.uniq!
  routines.empty?
}

And I admit freely that the inner inject was stolen from James’ try,
thanx ;).
Cheers
Robert

http://ruby-smalltalk.blogspot.com/

Whereof one cannot speak, thereof one must be silent.
Ludwig Wittgenstein

4

On Jan 12, 2008 10:52 PM, Robert D. [email protected] wrote:
Small improvement maybe

unused.delete_if{ |source, routines|
  routines.delete_if do |o|

— routines.inject(0){ |c,r| c + (r == o ? 1 : 0 ) } != MagicNumber
+++ routines.grep(o).size != MagicNumber

  end
  routines.uniq!
  routines.empty?
}

–
http://ruby-smalltalk.blogspot.com/

Whereof one cannot speak, thereof one must be silent.
Ludwig Wittgenstein

5

On Jan 12, 2008 11:09 PM, James G. [email protected] wrote:

On Jan 12, 2008, at 3:57 PM, Robert D. wrote:

— routines.inject(0){ |c,r| c + (r == o ? 1 : 0 ) } !=
MagicNumber
+++ routines.grep(o).size != MagicNumber

I’m kind of surprised that works. Neat.

However, I think inject() is preferable since it doesn’t have to build
up a possibly large Array just to get a count.
It depends, if space is the issue you are right, if time is an issue
inject is - much to my dispair - not the best idea.
The ideal solution of course is to use Ruby1.9 with count :slight_smile:
R.

–
http://ruby-smalltalk.blogspot.com/

Whereof one cannot speak, thereof one must be silent.
Ludwig Wittgenstein

6

On Jan 12, 2008, at 3:57 PM, Robert D. wrote:

— routines.inject(0){ |c,r| c + (r == o ? 1 : 0 ) } !=
MagicNumber
+++ routines.grep(o).size != MagicNumber

I’m kind of surprised that works. Neat.

However, I think inject() is preferable since it doesn’t have to build
up a possibly large Array just to get a count.

James Edward G. II

7

oh sorry I forgot, there is another potential performance problem in
my solution.
… look when I called uniq!.
I went for readability but just wanted to be honest about the
tradeoffs in case this is a problem for Bil.
R.

8

On Jan 12, 4:52 pm, Robert D. [email protected] wrote:

Maybe like this

unused.delete_if{ |source, routines|
  routines.delete_if do |r|
    routines.grep(r).size != MagicNumber
  end
  routines.uniq!
  routines.empty?
}

Thanks guys!

Again, the replies to my OP via news://comp.lang.ruby
did not come through my newsfeed. Sigh.

Regards,

9

2008/1/12, Bil K. [email protected]:

unused.each do |source,routines|
orphans = routines.uniq
orphans.delete_if do |orphan|
repeated = routines.find_all{ |routine| routine == orphan }.size
repeated != MagicNumber
end
unused[source] = orphans
end
unused.delete_if{ |h,k| k.empty? }

Here’s my solution, untested:

out = unused.inject({}) do |res, (k,v)|
array = v.inject(Hash.new(0)) do |ha, e|
ha[e] += 1
ha
end.inject([]) do |ar,(ke,cn)|
ar << ke if cn == MagicNumber
ar
end
res[k] = array unless array.empty?
res
end

Kind regards

robert

10

On Jan 12, 2008, at 10:24 AM, Bil K. wrote:

unused.each do |source,routines|
orphans = routines.uniq
orphans.delete_if do |orphan|
repeated = routines.find_all{ |routine| routine == orphan }.size
repeated != MagicNumber
end
unused[source] = orphans
end
unused.delete_if{ |h,k| k.empty? }

Here is my attempt. It uses Symbol#to_proc.

module Enumerable
def frequency
inject(Hash.new(0)) { |r, e| r[e] += 1; r }
end
end

def clense(data, magic)
data.inject({}) { |r, (src, rlist)|
l = rlist.frequency.select { |r, c| c == magic }.map(&:first)
r[src] = l unless l.empty?
r
}
end

data = {
:gone => [ 0, 1, 2 ],
:dups => [ 0, 0, 1, 1, 2, 2],
:onedups => [ 0, 1, 1, 2, 2, 2],
:threes => [ 0, 0, 0, 1, 1, 1, 2,2,2]
}

p clense(data, 2)