http://qs321.pair.com?node_id=219131


in reply to Tutorial: Introduction to Object-Oriented Programming

Too bad you are teaching people bad habits. I know using a hash reference to store you attributes in is common, but it's bad habit. Here on perlmonks, people scramble over each other if someone dares to post code that doesn't use 'strict' to scold at the person for not using strict.

Using hashes to store object attributes in is like writing non-OO code using global variables, no package names and certainly no strict. You're loudly declaring that you don't want the benefits of stricts, namespaces or scoping. On top of that, you're tossing one of the pillars of OO programming out of the window: encapsulation. You're not taking any steps to prevent hiding your implementation - it's even worse, you are forcing the classes that inherits you to adapt your implementation.

Let's look at some issues. Suppose we have an class, Animal whose instances have an attribute, species. We write two accessor methods, get_species and set_species:

sub set_species { my ($self, $species) = @_; $self -> {speceis} = $species; } sub get_species { my ($self) = @_; $self -> {species}; }

This code compiles fine. No matter how many warnings and strictness you have turned on. It will even run fine, at best you get somewhere down the line a warning that you are using an undefined value. The equivalent non-OO code wouldn't even compile with strictness turned on.

my $species; sub set_species { $speceis = shift; } sub get_species { $species; }

Same typo, but a compile time error.

And here's how you are not using proper encapsulation. Suppose you have a generic Document class. The generic class gives you some minimal functionality, like adding and deleting text. Suppose that the class wants to track the number of lines in the document. It's an attribute that's only used in this class, so it's even using the "convention" to put an underscore in front of the attribute name. The number of lines is stored in the attribute _lines. Now we subclass the generic class to create a LaTeX class, for LaTeX documents. As part of the functionality, we also keep track of the amount of lines of the generated dvi output. Since this will be a private attribute as well, we use _lines as attribute.

DONG DONG DONG! That will give problems. Sneaky problems, that won't be detected at compile time - and not even give warnings at run time, no matter how many warnings you turn on. Perl gives you name spaces, and scopes, but you aren't using them, not even in an OO program where encapsulation is one of the key elements.

Note that you have a similar problem in your code when you are using the underscore convention. You call _open as a method. Which means that if there's an inheriting class that uses the same convention, you call the subroutine in the class, not your own. Better call it sub as a sub, not as a method. Why would you? It's intended to be a private method, so you don't want to call an inherited method anyway.

The bottom line is, if you want to use object oriented programming, you're better off using a language designed by someone who understood the concepts of object orientness. And not a language that found a cute trick to use arrows.

Abigail

Replies are listed 'Best First'.
Re: Re: Tutorial: Introduction to Object-Oriented Programming
by jreades (Friar) on Dec 11, 2002 at 18:25 UTC

    Abigail-II, you make several pertinent points, but there are also a couple on which I'd like to hear a little more about your thinking before I make changes to the tutorial (which I'd be happy to do).

    1. I'm a little unclear as to whether you feel that I should simply have put in use strict in all my examples, or whether your are pointing out that using a hash_ref will not enable use strict to contribute much towards debugging, or both.

    I was trying not to clutter the tutorial with any code that was not directly relevant to the OO aspects of the tutorial, so while I agree that all of these examples should use strict I was rather on the fence as to whether it would just be a distraction to the reader. Putting it the way you have makes me think I should stop being lazy with my examples.

    2. With respect to hiding (full encapsulation via anonymous subroutines) I feel quite strongly that this belongs in an advanced OO tutorial such as this one. I don't believe that throwing closures into the mix will help people learn the fundamentals of how objects work in Perl.

    3. This ties into my feeling that, while hashes have their limitations, they are the easiest way to introduce someone to OO in Perl. Are they ideal? No. Are they less complicated than arrays or closures? Yes. Again, this was never meant to be the canonical OO tutorial, just an introduction.

    4. I realized that I was contradicting myself by saying that an inheriting class should never call a super class' private methods and then structuring my calls so that they *could* in fact call an inherited class' private method first, but wasn't thinking about the consequences of that decision in the way that you outlined them and was just trying to stick to the OO-style as a way of reinforcing usage.

    Anyway, I agree with you and I think that I will change my code to use subroutine calls rather than method calls as soon as I can.

    5. Use a different language -- I know what you're getting at here, but I think that pretending OO features don't exist in Perl is only going to lead to people doing BAD THINGS when they eventually discover it. People may pick up some bad habits here and there, but for the most part they (and those who have to maintain their code) will benefit from this feature more than they will suffer.

      I'm a little unclear as to whether you feel that I should simply have put in use strict in all my examples, or whether your are pointing out that using a hash_ref will not enable use strict to contribute much towards debugging, or both.
      No, I'm wasn't saying you should use strict in your examples. That would be pointless drivel. I'm pointing out that if you use hash keys as attributes you will not benefit from using strict. But using strict is a good practise so why advocate a coding style that cannot benefit from it.
      With respect to hiding (full encapsulation via anonymous subroutines) I feel quite strongly that this belongs in an advanced OO tutorial such as this one. I don't believe that throwing closures into the mix will help people learn the fundamentals of how objects work in Perl.
      Where did I say full encapsulation can only be done with anonymous subroutines? Full encapsulation doesn't belong in an advanced tutorial. Full encapsulation is where you start. It's just like driving a car. Avoiding pedestrians isn't "advanced driving" - it's part of the basics. And you can get full (data) encapulation with techniques no more difficult that using hash references.
      This ties into my feeling that, while hashes have their limitations, they are the easiest way to introduce someone to OO in Perl. Are they ideal? No. Are they less complicated than arrays or closures? Yes. Again, this was never meant to be the canonical OO tutorial, just an introduction.
      Did I say you need to use arrays or closures? No. Do I think you can avoid the limitations I pointed out only by using arrays or closures instead of hashes? No. I'd still use hashes.

      Here's how you can write the Quote class you used in your tutorial.

      package Quote; use strict; use warnings; my (%phrase, %author, %approved); sub new { bless [] => shift; # Any reference will do. } sub set_phrase { my $self = shift; my $phrase = shift; $phrase {$self} = $phrase; } sub get_phrase { my $self = shift; return $phrase {$self}; } sub set_author { my $self = shift; my $author = shift; $author {$self} = $author; } sub get_author { my $self = shift; return $author {$self}; } sub is_approved { my $self = shift; @_ ? $approved {$self} = shift : $approved {$self}; } sub DESTROY { my $self = shift; delete $phrase {$self}; delete $author {$self}; delete $approved {$self}; }

      Doesn't this remarkably look like your code? Perhaps my code is even simpler, I hardly use references - only the constructor returns a reference, but we don't care what it is, or how to put data in it (or to get something out of it). The only addition is the DESTROY function. But this code is using full encapsulation. Attributes cannot be trampled over. We're not enforcing our implementation on an inheriting class. We don't even care how an inherited class is implemented (if we are inherited, or we inherit something else, we don't need our constructor to be called - cool, isn't?)

      I'm convinced this way of implementing classes is actually easier to learn, and less error-phrone than the traditional "just dump everything in the same place" way by using a hashref.

      Abigail

        Abigail, your method of class construction is deceptively simple and (without trying it in anger) appears very robust.

        This is a bit like asking the vendor what is wrong with the house/car/London Bridge he is about to sell you, but

        Are there any weaknesses?

        If so, what are they?


        Okay you lot, get your wings on the left, halos on the right. It's one size fits all, and "No!", you can't have a different color.
        Pick up your cloud down the end and "Yes" if you get allocated a grey one they are a bit damp under foot, but someone has to get them.
        Get used to the wings fast cos its an 8 hour day...unless the Govenor calls for a cyclone or hurricane, in which case 16 hour shifts are mandatory.
        Just be grateful that you arrived just as the tornado season finished. Them buggers are real work.

Re^2: Tutorial: Introduction to Object-Oriented Programming
by adrianh (Chancellor) on Dec 12, 2002 at 00:07 UTC
    The bottom line is, if you want to use object oriented programming, you're better off using a language designed by someone who understood the concepts of object orientness. And not a language that found a cute trick to use arrows.

    I think that's a tad harsh :-)

    Most of the perl I write is OO. I have to take more care in some areas than I would in certain other languages, but it's still a win in terms of easy of production & maintainence than the equivalent non-OO perl.

    It's also a win over writing it in C++/Java/Eiffel/whatever because perl gives me other advantages in producing functional working projects in a timely manner.

    I'm not saying good encapsulation is not important - it is. But it's not the only advantage that OO gives you. I'm not even sure it's the most important one.

    Typos in hash keys can cause bugs, but they're simple bugs that are easy to track down and fix.

    Leading "_" characters to indicate "private" hash keys and method names are potentially more dangerous. Especially since they're normally excused with the line:

    "Perl doesn't have an infatuation with enforced privacy. It would prefer that you stayed out of its living room because you weren't invited, not because it has a shotgun."

    Which, while a colourful statement, hides the fact that the problem isn't forced entry, it's accidental trespass.

    That said, I've only encountered problems relating to these issues a couple of times in several years of (pretty complex) OO perl. It's rare you get deep inheritance, rarer still you encounter a name clash.

    I'm not saying it isn't a problem. It is. It can lead to some evil bugs that are hard to track down. One of the many reasons I'm looking forward to perl6 is that it's likely to clean up this mess. I'm just saying that it's not necessarily a common problem in many real world situations.

    There are, of course, many ways around these issues:

    • Your very own inside out objects (a lovely hack) cuts around the whole issue nicely. It also allows you to inherit from an evil hash object if necessary.
    • Call private subs as functions rather than methods to stop sub-classes accidentally overriding them.
    • Implement private subs as lexically scoped code references if you're really scared about people playing with them :-)
    • Use Class::Delegation to wrap classes that cause inheritence problems.
    • ... and so on ...

    However, I would question whether these ideas can be usefully be introduced into a tutorial aimed at novices.

    Hashes may not be ideal, but they're fairly simple to teach - and are familiar to perl coders from elsewhere. The kind of problems they introduce are not likely (in my opinion) to be much of a problem to the novice. Once you stop being a novice there are other techniques available that can solve the problems.

Re^2: Tutorial: Introduction to Object-Oriented Programming
by aufflick (Deacon) on Jul 30, 2004 at 02:26 UTC
    I have been bitten by exactly the bug you describe to argue!

    As mentioned elsewhere in the comments on your post, the only realy problem with your implementation of accessors is code maintenance - you need to add each get and set and then remember to add each store to the destroyer. I bet a lot of memory leaks would appear by forgetting to do this.

    A comment by fruiture suggests a single data repository in the parent object with single get and set methods that take attribute names, but that implementation uses a hash internally and leaves you open to the same problem. It does, however, address both maintenance issues.

    The same comment also points out that by avoiding a hash you lose the ability to use Data::Dumper to debug your objects easily.

    You could use this repository idea, but have a hash in your object specifying the name and type of each of the attributes, and run a check on every get and set call. As a bonus, you could also use this hash to drive data validation by specifying a data type and/or validator sub - the set function could then optionally run a data validation of requested.

      As mentioned elsewhere in the comments on your post, the only realy problem with your implementation of accessors is code maintenance - you need to add each get and set and then remember to add each store to the destroyer. I bet a lot of memory leaks would appear by forgetting to do this.

      See Class::InsideOut - yet another riff on inside out objects. for one way to avoid the chore of maintaining a DESTROY method.