Bug #112592 for Moo: Automatically define builder for lazy attrs

Mon Feb 29 23:16:59 2016 ZDM [...] cpan.org - Ticket created

Subject:

Automatically define builder for lazy attrs

Hi. I think - it will be handy to automatically add builder to the attr. spec. for lazy attributes without "default" and "builder" defined. Please, look at example: has attr => ( is => 'rw', lazy => 1 ); In this case "default" or "builder" are mandatory. And if "default" is not specified - it is obvious, that builder => 1 is missed.

Tue Mar 01 03:02:27 2016 ZDM [...] cpan.org - Correspondence added

So, I propose to add something like this into the attribute hash pre-processor: $spec{builder} = 1 if $spec{lazy} && !exists $spec{default} && !exists $spec{builder};

Tue May 17 13:33:25 2016 rt-cpan [...] trout.me.uk - Correspondence added

On Mon Feb 29 23:16:59 2016, ZDM wrote: Show quoted text

> Hi. > > I think - it will be handy to automatically add builder to the attr. > spec. for lazy attributes without "default" and "builder" defined. > Please, look at example: > > has attr => ( is => 'rw', lazy => 1 ); > > In this case "default" or "builder" are mandatory. And if "default" is > not specified - it is obvious, that builder => 1 is missed.

However, this means that you must write: has attr => (is => 'rw', lazy => 1); sub _build_attr { ... } rather than just: has attr => (is => 'rw', lazy => 1, builder => sub { ... }); so the already-supported syntax is shorter *and* doesn't require you to repeat the attribute name. So I'm not sure what you think such an addition would gain?

Tue May 17 13:33:27 2016 The RT System itself - Status changed from 'new' to 'open'

Tue May 17 17:21:35 2016 dzagashev [...] gmail.com - Correspondence added

Subject:	Re: [rt.cpan.org #112592] Automatically define builder for lazy attrs
Date:	Wed, 18 May 2016 00:21:13 +0300
To:	bug-Moo [...] rt.cpan.org
From:	"dzagashev [...] gmail.com" <dzagashev [...] gmail.com>

Hi. I am always write builders as separated subroutines, they can be overwrited in subclasses and this syntax seems more clear for me. That's why my proposition, seems, have a sense. On 17.05.2016 20:33, Matt S Trout via RT wrote: Show quoted text

> <URL: https://rt.cpan.org/Ticket/Display.html?id=112592 > > > On Mon Feb 29 23:16:59 2016, ZDM wrote:

>> Hi. >> >> I think - it will be handy to automatically add builder to the attr. >> spec. for lazy attributes without "default" and "builder" defined. >> Please, look at example: >> >> has attr => ( is => 'rw', lazy => 1 ); >> >> In this case "default" or "builder" are mandatory. And if "default" is >> not specified - it is obvious, that builder => 1 is missed.

> However, this means that you must write: > > has attr => (is => 'rw', lazy => 1); > > sub _build_attr { > ... > } > > rather than just: > > has attr => (is => 'rw', lazy => 1, builder => sub { > ... > }); > > so the already-supported syntax is shorter *and* doesn't require you to repeat the attribute name. > > So I'm not sure what you think such an addition would gain?

Tue May 17 17:25:18 2016 dzagashev [...] gmail.com - Correspondence added

Subject:	Re: [rt.cpan.org #112592] Automatically define builder for lazy attrs
Date:	Wed, 18 May 2016 00:24:57 +0300
To:	bug-Moo [...] rt.cpan.org
From:	"dzagashev [...] gmail.com" <dzagashev [...] gmail.com>

What the sense to use builder as anonymous sub? It should be replaced with default value. Or I am missing something? On 17.05.2016 20:33, Matt S Trout via RT wrote: Show quoted text

> <URL: https://rt.cpan.org/Ticket/Display.html?id=112592 > > > On Mon Feb 29 23:16:59 2016, ZDM wrote:

>> Hi. >> >> I think - it will be handy to automatically add builder to the attr. >> spec. for lazy attributes without "default" and "builder" defined. >> Please, look at example: >> >> has attr => ( is => 'rw', lazy => 1 ); >> >> In this case "default" or "builder" are mandatory. And if "default" is >> not specified - it is obvious, that builder => 1 is missed.

> However, this means that you must write: > > has attr => (is => 'rw', lazy => 1); > > sub _build_attr { > ... > } > > rather than just: > > has attr => (is => 'rw', lazy => 1, builder => sub { > ... > }); > > so the already-supported syntax is shorter *and* doesn't require you to repeat the attribute name. > > So I'm not sure what you think such an addition would gain?

Tue May 17 18:52:22 2016 haarg [...] haarg.org - Correspondence added

On Tue May 17 17:25:18 2016, dzagashev@gmail.com wrote: Show quoted text

> What the sense to use builder as anonymous sub? It should be replaced > with default value. Or I am missing something? > > > On 17.05.2016 20:33, Matt S Trout via RT wrote:

> > <URL: https://rt.cpan.org/Ticket/Display.html?id=112592 > > > > > On Mon Feb 29 23:16:59 2016, ZDM wrote:

> >> Hi. > >> > >> I think - it will be handy to automatically add builder to the attr. > >> spec. for lazy attributes without "default" and "builder" defined. > >> Please, look at example: > >> > >> has attr => ( is => 'rw', lazy => 1 ); > >> > >> In this case "default" or "builder" are mandatory. And if "default" > >> is > >> not specified - it is obvious, that builder => 1 is missed.

> > However, this means that you must write: > > > > has attr => (is => 'rw', lazy => 1); > > > > sub _build_attr { > > ... > > } > > > > rather than just: > > > > has attr => (is => 'rw', lazy => 1, builder => sub { > > ... > > }); > > > > so the already-supported syntax is shorter *and* doesn't require you > > to repeat the attribute name. > > > > So I'm not sure what you think such an addition would gain?

Specifying builder => sub { ... } installs the sub as _build_$attr and uses that as the builder. It's basically equivalent to builder => 1 and then sub _build_attr { ... }.

Wed May 18 04:10:52 2016 dzagashev [...] gmail.com - Correspondence added

Subject:	Re: [rt.cpan.org #112592] Automatically define builder for lazy attrs
Date:	Wed, 18 May 2016 11:10:38 +0300
To:	bug-Moo [...] rt.cpan.org
From:	"dzagashev [...] gmail.com" <dzagashev [...] gmail.com>

Again, if developer using following syntax for declaring attributes: has attr => ( is => 'rw', lazy => 1 ); it is obvious, that "default" or "builder" properties are missed and Moo should try to find standard builder method. Also, maybe new "is" type "rwl" can be added? Even "rwlp" ;-)? has attr => ( is => 'rwl' ); In any case, you should decide by yourself, want you to add this or not. I think, that this feature is useful, it easy to implement, will not break current behavior, will add additional syntax sugar and allows to write less code. I am using the following wrapper for "has": no strict qw[refs]; my $has = *{"$caller\::has"}{CODE}; no warnings qw[redefine]; *{"$caller\::has"} = sub { my ( $name_proto, %spec ) = @_; $spec{builder} = 1 if $spec{lazy} && !exists $spec{default} && !exists $spec{builder}; $has->( $name_proto, %spec ); }; On 18.05.2016 01:52, Graham Knop via RT wrote: Show quoted text

> <URL: https://rt.cpan.org/Ticket/Display.html?id=112592 > > > On Tue May 17 17:25:18 2016, dzagashev@gmail.com wrote:

>> What the sense to use builder as anonymous sub? It should be replaced >> with default value. Or I am missing something? >> >> >> On 17.05.2016 20:33, Matt S Trout via RT wrote:

>>> <URL: https://rt.cpan.org/Ticket/Display.html?id=112592 > >>> >>> On Mon Feb 29 23:16:59 2016, ZDM wrote:

>>>> Hi. >>>> >>>> I think - it will be handy to automatically add builder to the attr. >>>> spec. for lazy attributes without "default" and "builder" defined. >>>> Please, look at example: >>>> >>>> has attr => ( is => 'rw', lazy => 1 ); >>>> >>>> In this case "default" or "builder" are mandatory. And if "default" >>>> is >>>> not specified - it is obvious, that builder => 1 is missed.

>>> However, this means that you must write: >>> >>> has attr => (is => 'rw', lazy => 1); >>> >>> sub _build_attr { >>> ... >>> } >>> >>> rather than just: >>> >>> has attr => (is => 'rw', lazy => 1, builder => sub { >>> ... >>> }); >>> >>> so the already-supported syntax is shorter *and* doesn't require you >>> to repeat the attribute name. >>> >>> So I'm not sure what you think such an addition would gain?

> Specifying builder => sub { ... } installs the sub as _build_$attr and uses that as the builder. It's basically equivalent to builder => 1 and then sub _build_attr { ... }.

Wed May 18 15:59:37 2016 rt-cpan [...] trout.me.uk - Correspondence added

On Wed May 18 04:10:52 2016, dzagashev@gmail.com wrote: Show quoted text

> Again, if developer using following syntax for declaring attributes: > > has attr => ( is => 'rw', lazy => 1 ); > > it is obvious, that "default" or "builder" properties are missed and > Moo should try to find standard builder method.

Right, but rw+lazy+builder is a weird combination so I'm not sure what we'd gain by sugaring it, and, again, you've not explained why has attr => (is => 'rw', lazy => 1); sub _build_attr { return 'value' } is an improvement over the already supported, functionally identical, and shorter has attr => (is => 'rw', lazy => 1, builder => sub { return 'value' }); especially since the first one requires you to type the attribute name twice, thereby introducing more room for typos. What does a new slightly longer way to say the same thing gain you?

Wed May 18 16:19:03 2016 dzagashev [...] gmail.com - Correspondence added

Subject:	Re: [rt.cpan.org #112592] Automatically define builder for lazy attrs
Date:	Wed, 18 May 2016 23:18:37 +0300
To:	bug-Moo [...] rt.cpan.org
From:	"dzagashev [...] gmail.com" <dzagashev [...] gmail.com>

package A; use Moo; has attr => ( is => 'rw', lazy => 1 ); package B; use Moo; extends qw[A]; sub _build_attr ($self) { return 123; } On 18.05.2016 22:59, Matt S Trout via RT wrote: Show quoted text

> <URL: https://rt.cpan.org/Ticket/Display.html?id=112592 > > > On Wed May 18 04:10:52 2016, dzagashev@gmail.com wrote:

>> Again, if developer using following syntax for declaring attributes: >> >> has attr => ( is => 'rw', lazy => 1 ); >> >> it is obvious, that "default" or "builder" properties are missed and >> Moo should try to find standard builder method.

> Right, but rw+lazy+builder is a weird combination so I'm not sure what we'd gain by sugaring it, and, again, you've not explained why > > has attr => (is => 'rw', lazy => 1); > > sub _build_attr { > return 'value' > } > > is an improvement over the already supported, functionally identical, and shorter > > has attr => (is => 'rw', lazy => 1, builder => sub { > return 'value' > }); > > especially since the first one requires you to type the attribute name twice, thereby introducing more room for typos. > > What does a new slightly longer way to say the same thing gain you?

Wed May 18 23:34:59 2016 haarg [...] haarg.org - Correspondence added

On Wed May 18 16:19:03 2016, dzagashev@gmail.com wrote: Show quoted text

> package A; > use Moo; > has attr => ( is => 'rw', lazy => 1 ); > > package B; > use Moo; > extends qw[A]; > sub _build_attr ($self) { > return 123; > }

This leaves A in an essentially broken state. Why would you ever want to do that?

Thu May 19 02:00:18 2016 dzagashev [...] gmail.com - Correspondence added

Subject:	Re: [rt.cpan.org #112592] Automatically define builder for lazy attrs
Date:	Thu, 19 May 2016 09:00:02 +0300
To:	bug-Moo [...] rt.cpan.org
From:	"dzagashev [...] gmail.com" <dzagashev [...] gmail.com>

Graham, I don't use this constantly but, sometimes, this is useful in roles. On 19.05.2016 06:35, Graham Knop via RT wrote: Show quoted text

> <URL: https://rt.cpan.org/Ticket/Display.html?id=112592 > > > On Wed May 18 16:19:03 2016, dzagashev@gmail.com wrote:

>> package A; >> use Moo; >> has attr => ( is => 'rw', lazy => 1 ); >> >> package B; >> use Moo; >> extends qw[A]; >> sub _build_attr ($self) { >> return 123; >> }

> This leaves A in an essentially broken state. Why would you ever want to do that?

Fri May 20 12:20:52 2016 rt-cpan [...] trout.me.uk - Correspondence added

On Thu May 19 02:00:18 2016, dzagashev@gmail.com wrote: Show quoted text

> Graham, I don't use this constantly but, sometimes, this is useful in > roles.

However, in a role you need: has foo => (is => 'lazy', builder => 1); requires '_build_foo'; (the extends version you gave is just broken and should be a role) What I think you really want is to write an extension, MooX::AutoRequireBuilder or something, that converts has foo => (is => 'lazy'); into the above. In hindsight, I do wonder if it mightn't've been nice to have Moo::Role do the require-the-builder part by default but it's a little late in the day for that now.

Fri May 20 12:26:16 2016 dzagashev [...] gmail.com - Correspondence added

Subject:	Re: [rt.cpan.org #112592] Automatically define builder for lazy attrs
Date:	Fri, 20 May 2016 19:25:51 +0300
To:	bug-Moo [...] rt.cpan.org
From:	"dzagashev [...] gmail.com" <dzagashev [...] gmail.com>

ok. Thank you for replies. I already use this feature. I thought, that it can be useful for others. On 20.05.2016 19:20, Matt S Trout via RT wrote: Show quoted text

> <URL: https://rt.cpan.org/Ticket/Display.html?id=112592 > > > On Thu May 19 02:00:18 2016, dzagashev@gmail.com wrote:

>> Graham, I don't use this constantly but, sometimes, this is useful in >> roles.

> However, in a role you need: > > has foo => (is => 'lazy', builder => 1); > > requires '_build_foo'; > > (the extends version you gave is just broken and should be a role) > > What I think you really want is to write an extension, MooX::AutoRequireBuilder or something, that converts > > has foo => (is => 'lazy'); > > into the above. In hindsight, I do wonder if it mightn't've been nice to have Moo::Role do the require-the-builder part by default but it's a little late in the day for that now.

Fri May 20 12:47:42 2016 haarg [...] haarg.org - Correspondence added

Changing this would amount to a small compatibility break. Attributes that were previously working with an ignored lazy would break. We are considering adding this in the future though, along with having a builder imply a 'requires' on the builder sub. That will take additional testing though, so for now I'm adding a warning in the case that lazy is specified without a default or builder.

Fri May 20 12:50:36 2016 dzagashev [...] gmail.com - Correspondence added

Subject:	Re: [rt.cpan.org #112592] Automatically define builder for lazy attrs
Date:	Fri, 20 May 2016 19:50:19 +0300
To:	bug-Moo [...] rt.cpan.org
From:	"dzagashev [...] gmail.com" <dzagashev [...] gmail.com>

Thank you. On 20.05.2016 19:47, Graham Knop via RT wrote: Show quoted text

> <URL: https://rt.cpan.org/Ticket/Display.html?id=112592 > > > Changing this would amount to a small compatibility break. Attributes that were previously working with an ignored lazy would break. > > We are considering adding this in the future though, along with having a builder imply a 'requires' on the builder sub. That will take additional testing though, so for now I'm adding a warning in the case that lazy is specified without a default or builder.

Sun May 22 10:33:50 2016 rt-cpan [...] trout.me.uk - Correspondence added

On Fri May 20 12:26:16 2016, dzagashev@gmail.com wrote: Show quoted text

> I already use this feature.

Given every example you've given is shorter *without* it, I don't currently consider it a feature. Marking this ticket rejected until you can find something it's useful for.

Sun May 22 10:33:57 2016 rt-cpan [...] trout.me.uk - Status changed from 'open' to 'rejected'