Beefy Boxes and Bandwidth Generously Provided by pair Networks
There's more than one way to do things
 
PerlMonks  

new() function: incorrect?

by lukeyboy1 (Beadle)
on Nov 12, 2008 at 09:35 UTC ( #723088=perlquestion: print w/ replies, xml ) Need Help??
lukeyboy1 has asked for the wisdom of the Perl Monks concerning the following question:

Greetings, fellow monks. I was interviewed recently for a perl role, and they asked me to put together an object. I used my standard new() function (which I've been using for about, gosh, 10 years now), only to find that the interviewers thought it was wrong. Here's the code I wrote:
package Bob::MyObject; use strict; sub new { my $proto = shift; my $class = ref($proto) || $proto; my $self = { command => undef }; bless ($self, $class); return $self; }
I guess my question is this: what's wrong with this, and how can I improve it? (I'd always thought that this was a pretty standard way of putting a class together, and was very surprised by the interviewer's reaction, to be honest!).

Comment on new() function: incorrect?
Download Code
Re: new() function: incorrect?
by Anonymous Monk on Nov 12, 2008 at 09:53 UTC
Re: new() function: incorrect?
by fmerges (Chaplain) on Nov 12, 2008 at 09:53 UTC

    Hi,

    Well in itself it's nothing wrong with it, it creates an object...

    A couple of thinks can be mentioned like, there are people that don't like to have the possibility to instantiate a new object by calling an object see ref ||. Other things are more a style question... you don't need to have the bless and the return on two different lines. Maybe they didn't like the command => undef without the leading ',' to avoid errors when you add something and forget the comma...

    Another thing could be that you could skip the whole constructor even by using some sort of module, like Class::Accessor for example. The 'use warnings' pragma could be there, even commented it looks good...

    But it looks more that they evaluated overall your coding practices by this example... maybe to much BPP, who knows... I would rather look on a longer piece of code, with some algorithm, see if you know also to write tests, etc, etc.

    Regards,

    fmerges at irc.freenode.net

      Hi,

      Actually the worst "constructor" I've seen so far came from someone who considered himself an expert and it was like this:

      sub new{ my $self = shift; my $dbh = shift; $self->{dbh} = $dbh; return $self; }

      On top of that also calls like the following from the caller:

      $sDB->{dbh}->disconnect;

      And it was not in one module, it was in the whole layer that deals with the DB from a web application running under mod_perl! You can imagine for yourself what sort of strange creatures were also lurking around...

      Update: Added emphasis on the worst...

      Regards,

      fmerges at irc.freenode.net
        That won't even run. It dies of strictures. Definitely qualifies for worst.
Re: new() function: incorrect?
by ForgotPasswordAgain (Deacon) on Nov 12, 2008 at 09:54 UTC

    It's hard to know if there are contextual things missing, since I wasn't there. Did you ask them why they thought it was wrong? Maybe they were testing whether you really knew what you were talking about. Maybe they say it's wrong just to see how you react.

    Your new doesn't accept any arguments, maybe they didn't like that. You don't have a true value (1;) at the end of the package, maybe it was that. Maybe they didn't want you to actually write an object, but instead use a module like Moose.

      I should have added that this was just a snippet of code...not complete, like! :)

      "Did you ask them why they thought it was wrong?"

      Were I the interviewer I would most certainly raise a flag on this line:

      my $class = ref($proto) || $proto;
      This line is a classic "cargo cult" line that I myself used without knowing what it did or why it was there. The bottom line is that 9 times out of 10 this line will do what you want, but it is still cargo cult. I would have certainly preferred to see use Moose! :)


      jeffa

      L-LL-L--L-LL-L--L-LL-L--
      -R--R-RR-R--R-RR-R--R-RR
      B--B--B--B--B--B--B--B--
      H---H---H---H---H---H---
      (the triplet paradiddle with high-hat)
      
Re: new() function: incorrect?
by GrandFather (Cardinal) on Nov 12, 2008 at 09:55 UTC

    You may find the replies to confusion about blessing inside a constructor interesting.

    Bottom line: $class = ref($proto) || $proto; treats new as both a constructor and as a cloner. Better to have a separate clone method if you need one and simply:

    sub new { my ($class, %options) = @_; # validate options return bless \%options, $class; }

    which makes new a class method called by:

    my $obj = Bob::MyObject->new (option => 1, );

    but not as:

    my $newObj = $obj->new (option => 1, );

    Perl reduces RSI - it saves typing
Re: new() function: incorrect?
by JavaFan (Canon) on Nov 12, 2008 at 13:22 UTC
    Whether it's wrong or not depends on your viewpoint. I can identify at least three issues in your new methods I've seen people argueing against.

    1. People object to give the programmer liberty to create a new object of the same class as a given $obj by doing $new_obj = $obj->new, insisting the programmer must jump hoops and write $new_obj = (ref $obj)->new, because they themselves get confused, insisting that if an object method is called new it must return a clone.
    2. People object to integrating object construction with object initialization, arguing they are two different operations and should be into different function. There argument is that combining object construction with object integration makes it harder to do multiple inheritance. They argue it's better written as:
      package Bob; sub new {bless({}, shift)->init} sub init { my $self = shift; $self->{command} = undef; $self; } my $bob = Bob->new;
      That way, you can do MI more easily:
      package MyUncle; sub new {bless({}, shift)->init} sub init { my $self = shift; $self->{topping} = "dessert"; $self; } package BobIsMyUncle; our @ISA = ('Bob', 'MyUncle'); sub new {bless({}, shift)->init}; sub init { my $self = shift; $self->Bob::init->MyUncle::init; $self->{floor}='wax'; $self } my $bob_is_my_uncle = BobIsMyUncle->new;
    3. Some people say that using hashrefs as objects is wrong, as it leads to the possibility of name clashes of object attributes when inheriting (a subclass of Bob::MyObject may use 'command' itself as well). And that if you typo a hashkey name, you will not get a compile time error, just odd runtime behaviour. They prefer you use Class::InsideOut or some other module implementing classes for you.
    I typically ask how people prefer to implement their objects as well during an interview. Unless they really screw up, there aren't wrong answers. What interests me is the motivation (I ask for that). I don't care whether they have the same preference as I have, as long as they can motivate them (and bonus points for identifying the weaknesses in their choices).
      Their main point was "new() doesn't make the object -- it's not really OO programming, it's procedural"

      ..at which point, I said something along the lines of "I don't really agree with your thoughts on that", and left it at that.......

      (Honestly -- the most dangerous people are the ones that think that they DON'T need to ask questions of themselves, IMO -- what's the point, if you can't keep improving yourself?)

      To be honest, the panel of people also said that my commenting was "uninspiring" (but, what did they expect in a 45 minute interview, eh?!) -- but I like to declare an object like this (I'm not keen on the alternate ways of declaring a new object, but that's my own view!) :
      my $objectUsing = Bob::Object->new();

        "at which point, I said something along the lines of "I don't really agree with your thoughts on that""

        If your goal was to stand by your understanding then you certainly succeeded. However, if your goal was to get the job then you might have left a better impression by instead saying "Well I am always open to learning more correct ways of getting the job done." I am all for honesty but during the interview I am certainly aiming to please. If I disagree then I will reflect that by not accepting the offer. ;)

        jeffa

        L-LL-L--L-LL-L--L-LL-L--
        -R--R-RR-R--R-RR-R--R-RR
        B--B--B--B--B--B--B--B--
        H---H---H---H---H---H---
        (the triplet paradiddle with high-hat)
        
      I take point 2 on board. I think that's where they got hung up.

      I mentioned that I was using the new() function as a sort of short-hand to set up what properties that I might use for a simple example.

      but -- in the real world -- you'd nearly always have a separate init() function (or similair). I know I would, in most cases.

      but -- using $proto clearly stuck in the chap's craw. And you're always fighting a losing battle in that case, aren't you....?

      "That way, you can do MI more easily: "

      Friends don't let friends use multiple inheritance. ;) Down vote me all you want, i'll say it again and again: Friends don't let friends use multiple inheritance! I recommend looking into Roles and Traits if you want Bob to be yer uncle.

      jeffa

      L-LL-L--L-LL-L--L-LL-L--
      -R--R-RR-R--R-RR-R--R-RR
      B--B--B--B--B--B--B--B--
      H---H---H---H---H---H---
      (the triplet paradiddle with high-hat)
      
Re: new() function: incorrect?
by oshalla (Deacon) on Nov 12, 2008 at 13:58 UTC

    If they wouldn't explain or discuss, I'd say you were well out of it !

    I've always worried that the writer of the new() cannot control how it's called, given that any of:

    my $foo = Bob::MyObject->new(...) ; # or new Bob::MyObject ... my $foo = $bar->new(...) ; my $foo = Bob::MyObject::new(...) ;
    will invoke new(). The last can be distinguished if there are no arguments, and I've seen:
    my $class = ref($proto) || $proto || __PACKAGE__ ;
    recommended. But it's essentially ambiguous when arguments are present -- I wish for a prototype or other gizmo that would insert an undefined class/object argument when not called as a method.

    You can give sub new an empty prototype to ensure that any Bob::MyObject::new calls are either unambigious at run-time or errors at compile-time. (Remembering that method calls ignore prototypes.)

    I see the argument that $obj->new(...) is ambiguous, to the extent that, without checking what it does, the programmer doesn't know whether it's: (a) an instance of TMTOWTDI for Class->new(...) (or new Class ...); or (b) an overloading of new meaning clone. Perl Best Practices points out that $foo->new(...) may be a class or an object invocation, so argues that overloading is ambiguous and should be avoided.

    Conventions help us reduce complexity, so not overloading new looks reasonable to me. YMMV

    Now, if your new is never a clone operation, why would you want to $foo->new(...) -- unless $foo contains a class name (not an object) ? In the interests of, I think, simplicity, the modern advice appears to be to not accept $obj->new(...) -- so the common my $class = ref($proto) || $proto ; spell is deprecated (at least by some).

    Of course, if the programmer doesn't provide the right parameters to your object's new you may choose simply to dump them on their backside with a run-time error (deliberately or by omission) -- in some cases, you may have no choice. I could suggest that in the case of $obj->new(...) you have a choice. But, if you go around pandering to slack programmers who ought to know better, how are they going to learn ? Really.

    [In the context of cargo-cults: I guess you could say that not allowing for $obj->new(...) looks sufficient to eliminate slack practice -- whether it's necessary I leave open.]

      I must admit that whenever I've been coaching people on creating new objects, I almost always tell them to declare an object like this:
      my $objectBlah = Blah::Object->new();
      I guess my reasoning behind this is that you should always be clear on creating a new instance of an object. I can't think of too many instances where I'd do
      my $foo = $bar->new();
      ...that just wouldn't, generally, cut the mustard with me -- unless you *really* needed another instance of the same object to do other stuff with. As I write this, I was trying to think of when I've needed to do this exact thing, and I honestly can't think of any instances that I've done that.

      However -- that may merely be an indictment of how I like to design my objects and code, generally.....!

      Also, I, personally dislike the following syntax:
      my $foo = new Bob::Object;
      I can't think of an exact reason -- probably just that I think that perl's syntax is nearly always based around using something like (in the case of reading a constant item that you've set up in your object framework) :-
      my $constantStuff = Bob::Constants::CONSTANT_STUFF;
      Adding in something like this:
      my $bob = new Bob::Object;
      ...just doesn't fit (in my head at least) with the nice perl way of doing things.

      I know my reason doesn't make too much sense: I just like a bit of consistency........!

      PS I'm actually really enjoying this conversation about something as simple as the best new() function to use. It reminds me of why I love perl so much.......!
        Or even:
        Bob::Object::new();
        to correct my last example. It's a method, so deserves the use of -> as far as I'm concerned.

        \end belligerent rant! :)

        I wouldn't recommend:

        $obj->new(...) ;
        either. And, if I ever wanted to have a new object of the same class as another I'd prefer:
        ref($obj)->new(...) ;
        because (once you've worked out WTF is happening) it says exactly what is intended.

        However, on the general basis "Be strict in what you send, but generous in what you receive", throwing a run-time error at $obj->new(...) strikes me as ungenerous. Noting that perlobj for 5.10 (and 5.8.8) describes how to cope with this eventuality -- as the fourth in a sequence of increasingly complete object constructors.

Log In?
Username:
Password:

What's my password?
Create A New User
Node Status?
node history
Node Type: perlquestion [id://723088]
Approved by GrandFather
help
Chatterbox?
and the web crawler heard nothing...

How do I use this? | Other CB clients
Other Users?
Others drinking their drinks and smoking their pipes about the Monastery: (11)
As of 2014-08-01 16:42 GMT
Sections?
Information?
Find Nodes?
Leftovers?
    Voting Booth?

    Who would be the most fun to work for?















    Results (31 votes), past polls