Bug #51486 for Moose: defaults are applied before all args from new() are assigned

On Fri, 13 Nov 2009, Michael G Schwern via RT wrote: Show quoted text
Actually, you left a "lazy => 1" that makes your test pass. Show quoted text
The manual does mention this gotcha in several places and specifically says: When the "default" is called during object construction, it may be called before other attributes have been set. If your default is dependent on other parts of the object's state, you can make the attribute "lazy". -dave /*============================================================ http://VegGuide.org http://blog.urth.org Your guide to all that's veg House Absolute(ly Pointless) ============================================================*/

As Dave pointed out, this gotcha is mentioned in the manual. The order of attribute initialization is hash order and therefore not to be relied upon. If you need control over the ordering use lazy. Sorry this is surprising to you, but it is not a bug, it is documented behavior. - Stevan

On Fri Nov 13 07:01:29 2009, STEVAN wrote: Show quoted text
I don't see that in the 0.92 docs. Ahh, its in Class::MOP::Attribute. Of course. :/ Show quoted text
Instead of putting bigger signs around the pit, could we fill in the pit? Is there a good reason defaults aren't applied last?

On Fri Nov 13 19:45:50 2009, MSCHWERN wrote: Show quoted text
Sorry, yes you are correct. Show quoted text
Honestly, it is a pit people fall in once, perhaps twice, but probably no more then a handful of times before they learn that if you need control over this, you use lazy. We really try hard to balance between trying to not surprise new users and making sure advanced users are not hampered. You may not agree with the choices we make regarding this, and that is your right, but know that they were not made hastily and often times were pondered over days, weeks, months and even years. Show quoted text
Because that doesn't solve the problem, what if the other attribute you want to call also has a default? Trust me, we have thought this one through pretty extensively and we even had a discussion again about it this morning after I saw your bug. Trying to put a definitive and predictable order on attribute initialization is pretty hairy in the face of things like lazy attributes, roles, multiple inheritance, attribute overriding (+foo) and such. Sure it could be figured out, but we pretty much all agreed that the additional code and probable performance penalty would not be worth it when lazy exists and works exceedingly well in this capacity. Personally, I have a real aversion to having multiple features which are mostly similar but behave in subtly different ways. Which is pretty much what we would end up with since we almost certainly need some kind of lazy-like feature contained within the attribute initialization phase which would behave in subtly different ways from the real lazy. I find these types of things to be conceptual pits (to borrow you analogy) that are far more difficult for users to navigate and understand then something like "when you need control over the ordering, just use lazy". All this said, I will leave this bug open as a reminder for us to include better docs that explain the lack of init ordering and reasons why 'lazy' is the appropriate solution to that problem. Of course I would gladly accept a patch from you for this if you are so inclined. If you are still interested in having this type of feature I suggest you contact Jesse Luehrs (doy on IRC) or Yuval Kogman (nothingmuch on IRC) as they both spent some time pondering a possible MooseX:: module for this based on attribute traits, I am sure they would be happy to pass on their ideas to you. - Stevan

On Sat Nov 14 13:18:45 2009, schwern@pobox.com wrote: Show quoted text
Wrong example. What if A has a default and B has a default and neither are supplied and A depends on B? No circularity, but there's no way to know that B's default needs to be set before A's. Saying "let's just do defaults last" moves the problem instead of solving it.

On Tue Nov 17 01:53:42 2009, schwern@pobox.com wrote: Show quoted text
Nope, this is the entire reason you have to mark things lazy to get them to work properly. When name is called, it won't automatically populate the default unless the name attribute is lazy - otherwise, the initial problem still occurs. -doy

This is not a bug, you simply need to use (lazy => 1) and it will solve your problem, that is exactly the reason why that feature exists. Please use the mailing list next time, RT is not your personal Moose help-desk. - Stevan

Stevan Little via RT wrote: Show quoted text
Dude, I said from the beginning that setting lazy would "solve" it. If I just wanted to just make my code work I'd have left it at that. Users shouldn't have to set magic flags or study the documentation to make things work sensibly. They shouldn't have to set a make_it_work_right_but_a_little_slower flag (make_it_work_wrong_but_a_little_faster would be better). If you have to put caveats and warnings in the docs to steer the users away from the obvious choice of action that's a red flag and a source of user errors. I also like to report when I fall into pits so that later when someone else reports it they don't get "well you're the only one to report it so its not a problem". Bugs reports are like cockroaches, for every that gets reported there's a hundred you don't. Ten times that for interface problems which are technically not "bugs" and can be an uphill climb to convince the devs there's even a problem. I made the effort to dig into Class::MOP and figure out what's going on. I wrote my thinking out loud so maybe there could be a discussion or something. I submitted a simple patch which passes all tests and fixes a good chunk of the problem. I haven't been given a clear reason why it can't be applied as a partial solution. I didn't get an answer as to why lazy is not the default. If its for performance or just backwards compatibility, fine. Just say so. Until I know there's a good reason behind an awkward design choice I'll continue to pick at it. These are not personal help desk tickets. I found something hard to use, I want to make it easier for the next guy and a took a stab at patching it. That's what I do. That's what you're supposed to do. Not all bugs have an error message. -- 87. If the thought of something makes me giggle for longer than 15 seconds, I am to assume that I am not allowed to do it. -- The 213 Things Skippy Is No Longer Allowed To Do In The U.S. Army http://skippyslist.com/list/

Show quoted text
But that is the whole point, if you leave it out then your accessor will *not* be lazy and therefore will not exhibit the lazy behavior. We gave you a solution (actually you had it all along) that works perfectly and allows for room to grow even, but you decided you didn't want it, but wanted us to change core behavior instead. Show quoted text
Well to start with, lazy is not a "magic flag", it is a flag that indicates the generator accessor should behave in a certain way which is pretty clearly explained in the docs and is really not all that hard to understand. It is one of those features that straddles the line and will do easy things easily (like your use case) and allows much more complex behaviors such as the Tree example in the Cookbook (which prevents infinite recursion in a circular data structure). As for users studying the documentation, I disagree, I think if you want to learn how to use a powerful tool, you are going to need to read some docs. It is simply impossible for a tool as powerful as Moose to be so intuitive to everyone that close study of the documentation is not required at some point. Show quoted text
I could not possibly disagree more with that statement, making it "work wrong" but "a little faster" is not a compromise I am willing to take, I think my record on this is clear, I prefer correctness over speed anyday. And as with the "accessors should always be generated" discussion we had at YAPC, I would prefer that you get what you ask for, rather then having to ask *not* to get something. Also, your "right" and "wrong" are subjective and I disagree with your assessment, in fact I see them as the inverse. Show quoted text
I don't totally agree that it is "obvious" that default values will DWIM like you seem to think they should. But I think it has been pretty well established already that you and I disagree on what "DWIM" means so there is no need to belabor that. Show quoted text
Sure, and we took your report to heart and it spawned several discussions among the cabal members, but in the end it was decided that lazy was a far superior solution. To make what you wanted work properly would not only increase the object construction and accessor overhead, but add complexity to a part of the code which is already pretty complex. To solve this problem correctly would require a partial laziness to be implemented within object construction which would not only complicate the part of Class::MOP you patched, but also all the constructor and accessor code generation in Class::MOP and Moose. The end decision was that all this work, overhead and future maintanance was not worth it when lazy already existed and does the job perfectly and, as I said before, allows for room to grow. Show quoted text
And I appreciate that very much. Show quoted text
Right, but unfortunately you did it in an inappropriate forum, one which you have been asked repeatedly to not use in such a way. Discussion is for the mailing list or IRC where we can go over the details of the issue and get varied input from people /other then those who get the emails from RT/. If after that it is decided that it truely is a bug, then I am happy to have you put it into RT or re-open a closed or rejected bug. This is pretty much said/implied in the Moose::Manual::Contributing document as is the fact that we do not accept new features that are not first vetted by a MooseX:: module (which I suggested you do with this). Show quoted text
Actually, I disagree that it fixes the problem, or even a decent chunk of it, in fact I think that it creates more problems. Your patch solves your problem quite well, it does not solve the slightly more complex case Dieter (HDP) and Jesse (DOY) mentioned. Your partial solution would then require yet more caveats and workarounds to be added to the docs, something you yourself have said is a bad thing. Show quoted text
Yes, it is exactly that ... and more. We have tried very hard to make sure (at runtime) you only pay for what you use and making lazy the default would incur a penalty even on those who didn't ask for it. But it is not just about performance penalty, I believe, as I said above, that you should let the user decide and not make decisions for them. I find it very unintuitive to have to ask to *not* have a feature. Show quoted text
Again, we disagree on what DWIM means and what constitutes an "awkward design choice" so I don't expect to convince you. Show quoted text
Right, I don't disagree with that, I just think that RT is the wrong forum for *continuing* such a discussion, as I have stated to you already in the past (RT #34039 and #34313) and in my response to you in this ticket. Subscribe to the mailing list or visit us on #moose or even #moose-dev to discuss this further and pled your case. Show quoted text
And not everything *you* think is a bug is /actually/ a bug. In this case, it is your opinion that Moose is doing the wrong thing, not an indisputable fact, and I flat out disagree with you. Honestly our fundemental disagreement reminds me of the whole debate over the RoR philosophy of "convention over configuration". You are suggesting that Moose embrace "convention", while I am much more firmly in the "configuration" camp. As I am sure you will agree, configuration is the more flexible of the two, but at the cost of requiring explicitness from the user. While convention brings more consistency and percieved simplicity, at the cost of reduced flexibility. Moose is not like other CPAN libraries, it is a library with which to write other libraries with. In essence Moose's problem domain is not unlike that of a programming language. And as we all well know, programming languages are very subjective things, some people love Perl and find it very intuitive (warts and all), others hate it with a strong passion. Moose has already clearly exhibited this exact same ability to evoke strong emotions and opinions in others. In fact, I have always said that Moose is not for everyone and that no one should ever feel compelled to use it if they don't want. So, anyway, in conclusion (cause this is getting long), please take these things to the mailing list or IRC where they can get greater feedback and discussion, but do not be surprised when you ask us to change core behavior that we don't insist you vet this behavior in a MooseX:: module first. There is nothing worse then adding a feature only to find it has inheriant problems and then being stuck in situation of having to maintain backwards compatibility for a mistake . - Stevan