Perl is a wonderful language for it allows one to accomplish certain task in many ways imaginable. One other distinct feature of Perl is it's 'implementation' of the OOP paradigm. In my short career I have seen code where it was heavily abused, and as well rare few snippets where it made the most perfect sense.

Just recently, I was working on an older piece of code that I wrote some months ago. The reason I had to go back and look at the code was due to a just sprang bug that I had to fix. Although I could have taken significantly less time to repair the script, it took me much longer this time due to poor OO design of some of my objects which impaired my ability to fully discern program logic and consiquently fix the bug in the most efficient way (minimal required code change/impact). Here, I'll attempt to describe the nature of this OO design flaw on an imaginary script...


imagine class Gadget. 
package Gadget;

#
# 'build' new gadget...
# return undef on fail.
#
sub new {
  my ($proto, $name, %args) = @_;
  my $self = bless {}, $proto;
   
  $self->{name} = $name;

  . . . work with %args . . .

  return $self;
}

#
# check if a gadget has already been built.
#
sub is_built {
  my ($self, $name) = @_;

  return exists %_built_gadgets{$name};
}
1;

package main;

my $g1 = new Gadget('knob');

. . . create a bunch of other gadgets . . .

if ($g1->is_built($athing_name)) {
  print "Gadget '$athing_name' has already been built!!\n";
} else {
  . . . do something useful here . . .
}

[download]
Here the use and purpose of is_built() method is obscured by two factors. On the one hand, it seems that is_built() has something to do with the instance of a Gadget class (the $g1 object). Am I checking here whether object $g1 was built with name $athing_name? Or, do I simply check that an instance of class Gadget named $athing_name exists and that also it is somehow related to the $g1 object? On the other hand, there's this 'Gadget ... has already been built!' comment that gets printed on the success of the is_built() which sort of points me in the direction of thinking that the latter assertion is true (gadget named so-and-so exists).

Clearly, I had to change the way I used is_built() method in my code.

My immediate suggestion for improvement was to treat is_built() as a class method instead of as an per-object method. The problem is Perl is in no way as strict as C/C++ when it comes to these matters. Despite of this, I at times find myself frustrated over a piece of code that doesn't quite conform to common standards (logic). In this particular script, the 
if ($g1->is_built($athing_name)) {

[download]
should be instead written as 
if (Gadget->is_built($athing_name)) {

[download]
And also the is_built() sub has to be modified to this: 
sub is_built {   
  my ($class, $name) = @_;
 
  # return if dealing with a reference...
  # note: not tested ;)
  return if (ref $class eq '__PACKAGE__');

  . . .

  # do stuff if this sub is called ast a class
  # method.

  ...
}

[download]
since as far as the purpose and design of the imagined class goes, this method has no buisiness inside an object. When is used as a per-object method, it implies a weak (or confusing) logical statement. Frankly, I find it hard to frame original line in a clear statement of logic.

UPDATE: thanks Fletch for your comment, i fixed is_built() now (I forgot to update the post since I added a similar change to my script.. as i did in fact broke my script with the first implementation of is_built() :).

"There is no system but GNU, and Linux is one of its kernels." -- Confession of Faith

Replies are listed 'Best First'.
Re: poor Perl OO design hinders maintenance effectiveness..
by Fletch (Bishop) on Apr 29, 2002 at 17:29 UTC

    $subject =~ s/Perl //;

    If you modify is_built as you show at the end, it's no longer a class method and wouldn't work when called Gadget->is_built( $foo ), but we'll presume that's a typo :).

    If you really want to disallow calls on instances, check that the first argument isn't a reference (and you can even go as far as checking that it's not a reference and it isa('Gadget')). Perl is giving you the freedom to shoot yourself in the foot, or to post armed guards to shoot improper callers in thier feet.

[reply]
[d/l]
[select]
Re: poor Perl OO design hinders maintenance effectiveness..
by pdcawley (Hermit) on Apr 29, 2002 at 21:05 UTC
    I'm not entirely sure that forcing it to be a class method actually gains you all that much, generally I like the fact that the line between class and object methods is so blurred. Surely you'd be better spending your time coming up with a better name for the 'is_built' method which, as you rightly point out, is ambiguous when you call it as an object method.

    But naming is hard... has_cousin_named anyone?

[reply]
Re: poor Perl OO design hinders maintenance effectiveness..
by sfink (Deacon) on Apr 29, 2002 at 23:36 UTC
    I doubt there will be many objects blessed into the '__PACKAGE__' package. Try it without the quotes. Or better yet, without any string at all. And why be so gentle? What's wrong with
    die "you called me with a ".ref($class)." instance, you idiot!"
    if ref($class);

    [download]

    Actually, croak() or confess() would be better, since if you ever triggered that line you'd immediately want to know who called it.

[reply]
[d/l]
[select]

      Tell me about it, I've recently had to work with code where object constructors call completely unrelated methods instead of just creating the object. Some people just get carried away an make every damn thing an object. Nested objects also made the OO code completely un resuable and meant debugging had to drill down through each nested object.

      There is no point in OO if you don't use it so you can reap the benefits. Everyone should be taught how to use a gun before they fire it.

[reply]
        There's nothing wrong with making everything into an object, in fact there's a lot to be said for it. I've found that most maintainance headaches have arisen when the code is some bastardized mixture of OO and procedural code.

        I'm not sure what you mean by 'nested objects'. If you mean using delegation then I have to disagree, delegation is often a great way of getting well factored loosely coupled code with a minimum of dependencies. If you're talking about deep inheritance hierarchies (complete with the added fun of multiple inheritance) then yes, you have to be bloody careful with it.

        The thing about OO is that you have to do it wholeheartedly OO code can look somewhat... 'diffuse' to someone who is used to the procedural way of doing things and it can be hard to see where stuff actually gets done, but careful method naming combined with well 'composed' methods alleviate that. Especially when you've got a test suite to look at too...

[reply]

Back to Meditations