Moses Hohman
11/16/2004 1:47:00 PM
Ah, yes, you're right. Sorry, I was quoting my code from memory, not
actually looking at it. I had written unit tests for it and it seems to
work, unless I forgot a test (in particular it gets the correct answer
to the example you provide below). The correct citation is
def first_superset(elements)
detect { |array| elements.subset_of?(array) }
end
def subset_of?(other)
detect { |x| not other.include?(x) } ? false: true
end
which is slightly different than what I wrote before. It also does not
convert a boolean to a boolean, wihch I agree is superfluous : ) I
could also use nil?. Maybe that would be better.
I like your suggestion of requiring set, by the way.
Thanks to everyone for the many suggestions. I think that the best
solution depends on how this code will be used in the library in
question, and in this particular case, I'm either going to stick with
the goofy functional method because it incurs little change on the
codebase, or if my co-coders permit, I will define a custom class for
the arrays of arrays, since these arrays of arrays are special objects
(constants) in this particular problem.
Moses
On Nov 16, 2004, at 3:23 AM, Robert Klemme wrote:
>
> "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
>
>
>
>