Refactoring request

ruby-talk
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,

···

--
Bil Kleb
http://fun3d.larc.nasa.gov

2

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

···

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

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

3

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

···

On Jan 12, 2008 4:24 PM, Bil Kleb <Bil.Kleb@nasa.gov> wrote:

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

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

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

4

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

···

2008/1/12, Bil Kleb <Bil.Kleb@nasa.gov>:

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

--
use.inject do |as, often| as.you_can - without end

5

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)

···

On Jan 12, 2008, at 10:24 AM, Bil Kleb 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? }

6

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

···

On Jan 12, 2008 10:52 PM, Robert Dober <robert.dober@gmail.com> wrote:

      end
      routines.uniq!
      routines.empty?
    }

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

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

7

Thanks guys!

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

Regards,

···

On Jan 12, 4:52 pm, Robert Dober <robert.do...@gmail.com> wrote:

Maybe like this

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

--
Bil Kleb
http://fun3d.larc.nasa.gov

8

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

···

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

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

9

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.

···

On Jan 12, 2008 11:09 PM, James Gray <james@grayproductions.net> wrote:

On Jan 12, 2008, at 3:57 PM, Robert Dober 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.

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

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

10

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.