Which initialize method? (or, who is self?)

Hi. Consider the ruby-hmac gem:
http://pastie.org/580360

Line 81 makes the following call:
hmac = self.new(key)

But, insofar as I understand Ruby’s wonderful world context switching,
self, in the instance in which it is called, is both unnecessary
(because the method in which it is being called is already part of the
class) and incorrect (because the only initialize method available
requires more than one argument).

The file works (in some instances…), so I’ll assume my understanding
of Ruby is still poor. Which sucks because I thought I knew it fairly
well.

I’ve tried printing out self in each new context, and I get the expected
result (HMAC::Base). Except, that is, inside a Rails application running
on Ruby 1.9.1, in which case I get HMAC::SHA1 within the Base.digest
method. Awesome.

Any ideas?

On 11.08.2009 21:23, Daniel W. wrote:

Hi. Consider the ruby-hmac gem:
http://pastie.org/580360

Line 81 makes the following call:
hmac = self.new(key)

But, insofar as I understand Ruby’s wonderful world context switching,
self, in the instance in which it is called, is both unnecessary

On first inspection, yes. But not if you look closer:

(because the method in which it is being called is already part of the
class) and incorrect (because the only initialize method available
requires more than one argument).

The method invoked is #new and not #initialize! This means that
argument lists of new and initialize need not be identical. You can
create a class whose Method.new has different arguments than
#initialize:

class Foo
def self.new(x)
o = allocate
o.send(:initialize, x, 1, 2)
o
end

def initialize(a, b, c)
# …
end
end

In this case it works differently though:

The file works (in some instances…), so I’ll assume my understanding
of Ruby is still poor. Which sucks because I thought I knew it fairly
well.

I’ve tried printing out self in each new context, and I get the expected
result (HMAC::Base). Except, that is, inside a Rails application running
on Ruby 1.9.1, in which case I get HMAC::SHA1 within the Base.digest
method. Awesome.

Any ideas?

The fact that the class is called “Base” indicates that it is intended
for inheritance. I don’t have a Ruby interpreter at hand right now to
cook up a tested example, but considering inheritance and the fact that
class methods are inherited by subclasses the whole thing actually makes
sense:

class SHA1 < Base
def initialize(key)
super(SHA1, 1024, 4096, key)
end
end

Now line 99 makes sense in combination with 81 / 91: Base.new is private
which will make self.new() trigger an exception and thus prevent
invocation of Base.digest() and Base.hexdigest(). But, you can invoke
SHA1.digest() and it will work.

Arguments algorithm, block_size and output_length to Base.initialize are
actually class dependent, i.e. every subclass likely defines their own
values.

Kind regards

robert

On Tue, Aug 11, 2009 at 10:10 PM, Robert
Klemme[email protected] wrote:

On 11.08.2009 21:23, Daniel W. wrote:

Well analyzed, I did not really get it.

I ask myself if this is not a violation of one of the principles of OO
design? Base.hexdigest assumes that the implementer of subclasses’
new will do the right thing, that is call super( ) with the right
params or implement allocate and send :initialize the right way.
This is clearly a superclass relying/imposing subclass behavior, not
what I learnt. Well I do not say it must not be done, but at least it
merits prominent documentation, so that less inspired folks than you
Robert, that is poor OP and me ;), get it too.

Maybe this shows better what I mean
http://pastie.org/580961

Cheers
Robert

On Wed, Aug 12, 2009 at 9:25 AM, Robert
Klemme[email protected] wrote:

design? Base.hexdigest assumes that the implementer of subclasses’
constraints on its subclasses - just think of the template method
pattern.
I was afraid you would say this ;). So I conclude that you indeed see
it somehow like this I could not make my mind up.
Correct me if I am mistaken but the template pattern is an abstraction
over differently implemented details, and indeed that seems to be the
case here. What still worries me is that the implemented details shine
through, I would rather hide them away. But right now I am confused
about the way I would prefer the responsibilities defined here.
I believe it is crucial for easy maintainable design, but right now I
am a little bit confused.
Probably I do not like the idea of the factory being in the base
class. Maybe because this is too Java like ;). Gotta think that over
again.

Well I do not say it must not be done, but at least it
merits prominent documentation, so that less inspired folks than you
Robert, that is poor OP and me ;), get it too.

I agree: if the original pastie covered the complete code then
documentation is really lacking some explanation. Even if class Base
is just intended for library internal usage, documentation would help
the maintainer.
It all boils down to documentation I agree.
increases when adding optional properties to the sub class (the # of
constructors doubles).
Pitty indeed;

Thx for your time.
R.

2009/8/12 Robert D. [email protected]:

This is clearly a superclass relying/imposing subclass behavior, not
what I learnt.

It is quite common for a base class to impose certain behavior related
constraints on its subclasses - just think of the template method
pattern.
I was afraid you would say this ;). So I conclude that you indeed see
it somehow like this I could not make my mind up.

:-)))

Correct me if I am mistaken but the template pattern is an abstraction
over differently implemented details, and indeed that seems to be the
case here.

Yes, I think that is an appropriate description.

What still worries me is that the implemented details shine
through, I would rather hide them away. But right now I am confused
about the way I would prefer the responsibilities defined here.

Another option would be to require a sub class to exhibit this
information via methods or constants. Those could be queried as
easily as the “constructor” argument but they would be placed where
they belong conceptually (in the class).

I believe it is crucial for easy maintainable design, but right now I
am a little bit confused.

Yes, the design is not obvious.

Probably I do not like the idea of the factory being in the base
class. Maybe because this is too Java like ;). Gotta think that over
again.

Hmm… For me the problem is more in the way how the initialize
methods are built. Since algorithm, block_size, output_length are not
passed in on the call they must be provided by the sub class (i.e. the
implementation of sub class #initialize methods). If they are not
somehow derived from the input (which is only “key”) or otherwise
dynamically generated they could be as well class constants.

Well I do not say it must not be done, but at least it
merits prominent documentation, so that less inspired folks than you
Robert, that is poor OP and me ;), get it too.

I agree: if the original pastie covered the complete code then
documentation is really lacking some explanation. Even if class Base
is just intended for library internal usage, documentation would help
the maintainer.
It all boils down to documentation I agree.

Documentation would certainly help but I feel a little itch from the
design (see above). So it’s probably a bit of both.

Thx for your time.

You’re welcome! I am wondering what Daniel makes of this…

Kind regards

robert

2009/8/12 Robert D. [email protected]:

On Tue, Aug 11, 2009 at 10:10 PM, Robert
Klemme[email protected] wrote:

On 11.08.2009 21:23, Daniel W. wrote:

Well analyzed, I did not really get it.

Thank you! It took me some time to see through it, too.

I ask myself if this is not a violation of one of the principles of OO
design? Base.hexdigest assumes that the implementer of subclasses’
new will do the right thing, that is call super( ) with the right
params or implement allocate and send :initialize the right way.

I would say Base#initialize is the one to “assume” something, namely
that SubClass#initialize invokes it with the proper argument list.

This is clearly a superclass relying/imposing subclass behavior, not
what I learnt.

It is quite common for a base class to impose certain behavior related
constraints on its subclasses - just think of the template method
pattern. Every abstract method forces subclasses to implement it,
which need to be instantiated. Every other method that you want to
override in a subclass has particular arguments that the overriding
method in the subclass is usually bound to (technically not in Ruby,
but even there it is probably a good idea to stick with the argument
list if you want to use the sub class in places where the super class
was used).

Well I do not say it must not be done, but at least it
merits prominent documentation, so that less inspired folks than you
Robert, that is poor OP and me ;), get it too.

I agree: if the original pastie covered the complete code then
documentation is really lacking some explanation. Even if class Base
is just intended for library internal usage, documentation would help
the maintainer.

Maybe this shows better what I mean
http://pastie.org/580961

IIRC in “Java Pitfalls” there is a chapter about how to get
constructors right in light of inheritance (Item 8: Design
Constructors for Extension) . Unfortunately I do not remember the
details nor do I have a copy of the book available (too bad it’s out
of print) but the statement which was put forward mandated that
classes with only default constructors were preferable. It may have
to do with the fact that in Java every subclass should then also
provide constructors with the same argument list as the super class
constructors and that the number of constructors dramatically
increases when adding optional properties to the sub class (the # of
constructors doubles).

At the moment I can think of only one restriction to this rule of
thumb: immutable classes, mandatory arguments and immutable arguments
for which you do not want to provide a setter. In both cases it does
make sense to provide the data via constructor arguments, because then
you can ensure that all instances are created properly by raising an
exception in case wrong data was passed in.

Kind regards

robert