It has taken me a while to come around to the position that using code such as that in the title (or similar) is really cargo-cult programming. Given that seems to be the case, then I was somewhat dismayed to see that Moo automatically includes such when it generates a constructor.
I initially got around this by using before new to detect the presence of a ref instead of a string and issuing an appropriate croak message. (tip of the hat to the folks on #moose for that!)
However, several others on #moose, notably some of the authors of Moo, said that was not a wise practice, and indeed, I found that the code was position dependent in relation to where BUILDARGS is defined. Specifically, the code for before new must come after BUILDARGS is defined or else the return value from that gets blown away.
After asking a couple of questions, and getting the expected response of "Well, write it yourself!" I decided to do just that. I have a working patch to Moo that disables the code in question, replacing it with an exception.
My question at this point is this: Is the ability to force a constructor to be a "class only" method a viable feature to have in Moo (and be extension, Moose)?
The solution I arrived at after reading the code for Moo and a few other MooX modules led me to use an option passed into Moo's import method. This was mainly due to the fact that other than a significant refactoring of Method::Generate::Constructor it does not seem feasible to alter the code being generated.
While I am waiting for a review of my proposed patch, I'd like to hear the thoughts of my fellow monks if this is a desirable option to have.
You must always remember that the primary goal is to drain the swamp even when you are hip-deep in alligators.
Re: $class = ref($class) if ref($class) Redux (theory)
by tye (Sage) on Jul 04, 2014 at 15:04 UTC
|
In theory, $obj->new( ... ) is ambiguous and should be avoided. Very often, in practice, the meaning of $obj->new( ... ) is obvious and useful.
The bigger mistake is using a method generator, especially one that leaks information that should be internal to the implementation (the attribute names) to the external interface (including the constructor).
A bigger mistake than that is even contemplating "before" hooks. And some (but not all) of the reasons why they are a horrible method of abstraction are actually demonstrated above.
But, yes, I have several times implemented class-only (and even more likely, object-only) methods. I'm most likely to do that by having a different package for class methods vs object methods.
I encourage you to take a step back and give more thought to your primary goals and reassess your secondary goals. In this context, OO purity is a secondary goal, one meant to be a shortcut to evaluating likely usefulness toward primary goals, mostly modular design.
Moose is anti-modular. If you avoid all of the anti-modular features of Moose (which you can), then you are left with no benefits (but real overhead costs).
| [reply] |
|
Over the years I have found that I have rarely, if not never, had occasion call $obj->new() if for no other reason than the desired result is usually ambiguous. Granted for many years I put in the usual code under discussion because I thought it might be handy to get the class name from an object.
Today, I almost always put in code such as this:
croak 'Must be called as a class method' if $ref($class)'
I also often use something like this in methods:
croak 'Must be called as an instance' unless ref($self)
(And I generally put in param count and validation checks that croak.) In my experience this saves a lot of time tracking down bad code, especially in a multi-team environment.
What annoyed me was that I could not use that code in Moo's BUILDARGS due to the code being generated in the constructor. In fact, the only way to accomplish that was to use before new, which has been pointed out by many is not a good practice. In this particular case, I don't think it has much of a downside, but I do think Moo (and by extension Moose) should allow for generating a constructor that takes that into account.
The goal is not OO purity, but rather, easy to understand code that someone can grasp a year from now. On a side note, I have long maintained that one cannot fully appreciate just what Moo is doing without a good understanding of classic OO Perl. I can readily see what you mean when you say Moose is anti-modular. I would be more inclined to say that there are those who have taken modularity to an extreme with it and have lost sight of a project's architecture in the process.
By the way, I have avoided Moose for a long time because of the size and performance issues that we all know so well. However, I am favorably impressed by Moo on both counts. It provides the basic functionality that one wants from Moose, and has surprisingly acceptable performance. The parts of Moose that Moo is missing actually seems like a Good Thing(tm) to me as it tends to prevent one from getting carried away with unneeded complexity.
You must always remember that the primary goal is to drain the swamp even when you are hip-deep in alligators.
| [reply] [d/l] [select] |
|
I can readily see what you mean when you say Moose is anti-modular. I would be more inclined to say that there are those who have taken modularity to an extreme with it and have lost sight of a project's architecture in the process.
Please could you give an example to clarify what you mean by this?
| [reply] |
|
|
At $work, our coding rules require that the constructors be named 'new', 'copy' and 'clone', so we have no ambiguity - in our code. Beyond that, these names seem very clear in their intent, so the first time I encountered a class where new made either a copy or a clone, I was very surprised.
| [reply] |
|
For a constructor of an object based on another object, 'copy' and 'clone' sound like the same verb to me. So it is nice that this is "very clear" for you in your context. With the context I have, I can't imagine what your 'copy' does.
The only ambiguity with $obj->new( ... ) is when the "..." part is incomplete. In that case it isn't clear if, for the parts not mentioned, whether you get empty (or "default") versions of those or versions based on what $obj had. So, as I said, in theory, $obj->new( ... ) is ambiguous. But there are many cases where the meaning is pretty obvious (in context).
For example, you can sometimes significantly simplify a class by not allowing partially constructed objects (to the point that I've seen that as a "best practice" in some environments). So you can have new() where not specifying part of the object is fatal. So it is obvious that nothing in $obj is going to override everything that is explicitly specified in the arguments to $obj->new( ... ).
A pretty natural pattern (particularly in a language like Perl and particularly in a place like CPAN) is to have a class that supports a number of "programmer preferences" (whether exceptions are thrown, use of whitespace in output or construction of strings, strict standards compliance vs. allowing convenient extensions, etc.). So, each object might have very little actual "instance" data but the user of the class can set a number of "options" to fit their particular environment or just personal preferences. So it makes sense to allow the user to create a factory object from which to create their objects. The factory would be configured with their particular preferences so that they don't need to specify those each time they construct a new object.
But once you get to that point, you likely also realize that the simplest way to achieve that is to just let the class be its own factory class. Then $obj->new( ... ) is the obvious syntax for using the preferences from $obj when constructing a new object. The only ambiguity is if $obj was given more than just preferences while ->new(...) was given nothing but preferences. But that's also in theory. In practice, details of the specific class might make it quite clear what happens in that case (or make that case impossible).
To be clear, I'm not arguing that it is inappropriate for a collection of somewhat related classes (such as those produced by a single team) to decide to studiously avoid $obj->new( ... ). Avoiding that can be a reasonable "best practice", especially in some environments.
If your framework prevents you from doing something so dead simple as croak()ing if the item passed to new() is a reference, then I think you should seriously evaluate your framework. There are places where generating methods can be a reasonable choice. But using a generated method should not be required. If your getter or setter has work to do other than just copying one scalar value uninspected, then I suggest you don't generate that particular getter or setter (just write it out).
If your framework does not even give you the option of writing your own new(), then that seems like a serious limitation (I don't know if Moo[se] precludes you from doing so or just causes you to lose too many "benefits" were you to do that). Of course, having a constructor that exposes all of your attribute names in its interface is also a horrible idea from a design perspective and is quite anti-modular, so I don't want the constructor Moose generates anyway. :)
Yeah, unconditional method generation sucks and can prevent you from doing trivial things or require you to do problematically complex work to accomplish what should be trivial.
| [reply] [d/l] [select] |
|
Those are some good coding rules. Methods should do one thing. megaPolyConstructorThatDwimsWhenItGuessesRight() constructs objects and confusion.
| [reply] [d/l] |
Re: $class = ref($class) if ref($class) Redux
by Anonymous Monk on Jul 04, 2014 at 07:08 UTC
|
| [reply] |
Re: $class = ref($class) if ref($class) Redux
by Bloodnok (Vicar) on Jul 04, 2014 at 06:55 UTC
|
| [reply] |
|
|