"The Ruby Way" and adding methods to base classes in libraries

M

Moses Hohman

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?

thanks in advance for any comments,

Moses
 
F

Florian Gross

Moses said:
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?

IMHO it is currently the above for very special methods and core class
additions for more general methods. In Ruby2 there will be namespace
selectors which will let you localize changes to core classes. IIRC
David Black already implemented something similar for current Ruby, but
I'm not sure if it is thread safe.
 
J

Joel VanderWerf

Moses said:
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?

You could define this method in a module and use Object#extend to give
that functionality to particular arrays.

It doesn't completely prevent conflicts, but it does limit the instances
that might be affected.
 
T

Tim Bates

Joel said:
You could define this method in a module and use Object#extend to give
that functionality to particular arrays.

The other way to deal with this would be to create a new class that
inherits from Array:

class SuperSetArray < Array # or some other, better name
def first_superset_of(*arrays)
arrays.detect do |array|
self.detect do |element|
not array.include?(element) ? false : true
end
end
end
end

Tim.
 
J

Joel VanderWerf

Tim said:
The other way to deal with this would be to create a new class that
inherits from Array:

class SuperSetArray < Array # or some other, better name
def first_superset_of(*arrays)
arrays.detect do |array|
self.detect do |element|
not array.include?(element) ? false : true
end
end
end
end

This is useful, but when an Array gets returned from, say, collect, it's
not of this class, so you may still want to extend with a module that
defines the same method.
 
R

Robert Klemme

Moses Hohman said:
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=> ["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
 
M

Moses Hohman

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

Moses Hohman said:
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=> ["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
 

Ask a Question

Want to reply to this thread or ask your own question?

You'll need to choose a username for the site, which only take a couple of moments. After that, you can post your question and our members will help you out.

Ask a Question

Members online

No members online now.

Forum statistics

Threads
473,769
Messages
2,569,578
Members
45,052
Latest member
LucyCarper

Latest Threads

Top