"Moses Hohman" <mmhohman@northwestern.edu> schrieb im Newsbeitrag
news:3A44CC1B-3744-11D9-969A-000393DB7722@northwestern.edu...
I have a question about practical experience with adding methods to
base classes in libraries. Say I'm writing a library and in a method of
that library, I need a method like "first_superset_of", which acts on
an array of arrays, and returns the first such array that is a superset
of a second array argument's elements, i.e.
def first_superset_of(arrays, elements)
arrays.detect do |array|
elements.detect do |element|
not array.include?(element) ? false : true
end
end
end
private :first_superset_of
Now, it's not very object-oriented to add this as a private method in a
class that is not an array or an enumerable or whatever and then call
it on two arguments. It seems more the ruby way to actually add a
method called first_superset_of (and maybe a method called subset_of?
for the second inner loop in there) to Enumerable or to Array, and then
use that method that way. However, adding this method might clash with
another added method in another library that a user might be using, if
the author of that other library also created a method on Array or
Enumerable with the same name.
So, what's the right way to handle this situation?
This methods seems too special to include it into Array IMHO. I'd leave
it as standalone impl (in fact, it's merely a function since it does not
use local state).
What I'd *do* change is the implementation. As far as I can see, your
original implementation does not yield the expected result:
a2 = [%w{a b c d}, %w{a b c d e}, %w{a b c}, %w{a b c d e}]
=> [["a", "b", "c", "d"], ["a", "b", "c", "d", "e"], ["a", "b", "c"],
["a", "b", "c", "d", "e"]]
a = %w{a b c e}
=> ["a", "b", "c", "e"]
def first_superset_of(arrays, elements)
arrays.detect do |array|
?> elements.detect do |element|
?> not array.include?(element) ? false : true
end
end
end
=> nil
first_superset_of( a2, a )
=> ["a", "b", "c", "d"]
This answer is clearly wrong as "e" is not included in the result.
Apart from the fact that using "?:" to convert a boolean into a boolean is
quite superfluous there's a logic error: you find the first element in
elements that is not included in the current array and since that's
usually not nil or false you select the current array from arrays.
Usually you will get the first element of arrays back.
You'll rather want this implementation:
def first_superset_of(arrays, elements)
arrays.detect do |array|
elements.all? {|element| array.include? element}
end
end
You can also do this:
require 'set'
def first_superset_of(arrays, elements)
master = elements.to_set
arrays.detect do |array|
array.to_set.superset? master
end
end
This might be more efficient, depending on the size of arrays involved.
Also it has the added advantage of directly expressing what you want.
Kind regards
robert