Re: object accessors have been getting invoked as $class->foo()

On Sep 28, 2008, at 01:23, Eric Wilhelm wrote:

> Right.  So that's part of why I like many small patches.  Something  
> like that makes the test pass, but it usually points to a bigger  
> problem.

My patch was small. It did only two things:

  * Add support for params to add_property()
  * Add support for coderef defaults to add_property().

The latter could easily be rolled back if you had wanted them  
separately. The scoping of `no strict 'refs'` was a simple refactor I  
could do while I was in there.

>> I think it needs to just go with "no strict 'refs'" in each of those
>> blocks (with a comment mentioning "class method"!) and then we'll
>> triage the symbolic ref issue in another go.
>
> Well, it appears that they only ever get called as $class->quiet()  
> and $class->get_options(), both of which are our own fault.

Right. I looked briefly at trying to fix that, but didn't have any  
luck. It seems that by allowing the `no strict 'refs'` bit, the  
properly can continue to be returned after it has been set. By not  
using it, and constructing a hashref each time, of course it is not  
set after the first time.

> As for the rest of the world...
> We'll have to see what we discover during the alpha run and perhaps  
> we'll find some stale copy+pasted methods there, but for now (I  
> think perhaps a release or two) I think the thing to do is to carp()  
> and return undef.  In the long run, it should either croak() or do  
> something useful.

Yeah. I doubt that there will be real-world problems. add_property()  
was undocumented, and the class interface to it was even less clear (I  
had no idea until tests started failing with my refactor).

> That is, If I'm reading this correctly, anybody using $class- 
> >foo('bar') as a setter is not going to get 'bar' back in $class- 
> >new->foo() anyway (because it went in $Module::Build{foo} = 'bar' -  
> which is afaict not referenced during object construction), so it  
> seems safe to "break this unexpected behavior".

That's what I thought, too. But the `no strict 'refs'` bit allows it  
to actually be returned~

% perl -MModule::Build -le 'Module::Build->installdirs("foo"); print  
Module::Build->installdirs'
foo

Crazy hack, eh? Ken, was there a good reason for this?

> As for the rest of your patch David, would you mind porting it  
> forward to fit this refactor?  I think the API you'll want is  
> basically like: _make_accessor($property, %opts).

Yes, I'll do that, but I'm less inclined to contribute in the future  
if you force me to rewrite my patches because you discover, thanks to  
my patch, that you don't like the current code, so you change it and  
force me to rewrite. Please, if you're going to do that, wait until my  
code is committed so that there's less work to be done overall.

Best,

David