Re: [CDBI] new reserved words in 3.0.11?

> On 4 Nov 2005, at 11:36, Patrik Wallstrom wrote:

On Fri, 2005-11-04 at 12:36 +0000, William Ross wrote:
> On 4 Nov 2005, at 11:36, Patrik Wallstrom wrote:
> 
> > Did anybody understand what I meant in this mail?
> >
> > With the example objects below in version 3.0.11, I can't access
> > method $object->name or $object->type, only those accessors which
> > actually changed name in accessor_name_for.
> 
> I think this may be because you're changing the accessor name (by  
> lowercasing it) but not the mutator name. If they differ, cdbi will  
> create separate get and set methods rather than a single mutator  
> method. This comment is in the code:
> 
>    # Make a set of accessors for each of a list of columns. We construct
>    # the method name by calling accessor_name_for() and  
> mutator_name_for()
>    # with the normalized column name.
> 
>    # mutator name will be the same as accessor name unless you  
> override it.
> 
>    # If both the accessor and mutator are to have the same method name,
>    # (which will always be true unless you override mutator_name_for), a
>    # read-write method is constructed for it. If they differ we  
> create both
>    # a read-only accessor and a write-only mutator.
> 
> but I don't think it's exactly right.

I agree. It looks like a new bug to me. I think the comments agree with
the POD and with the way accessor_name() used to behave. i.e. they're
correct :)  But as far as I can see, the code now does something
different.

>  the accessor and mutator names  
> are set when the column object is constructed, then  
> _mk_column_accessors calls $col->accessor($class->accessor_name_for 
> ($col)), which in your case *will* mean that the accessor name  
> changes. The comment suggests that this will only happen if you set  
> the mutator name, which is wrong.

I think your interpretation of the comment is wrong here. The idea is
that in simple cases where you just want to change both the accessor and
mutator to have a new name (e.g. customer instead of customer_id) you
just override accessor_name_for and it changes the names of both the
accessor and mutator identically. I think that's what the comment says
(and what the POD says and how CDBI 0.96 behaves).

But I don't think the code does that. It appears to set the default
values separately in CDBI::Column->new, using different hash keys. So
$col->accessor is 'customer_id' and so is $col->mutator. Then you
override accessor_name_for so it returns 'customer', and so the value of
$col->accessor is changed to 'customer' as well. But $col->mutator isn't
changed, and you haven't overridden mutator_name_for so the mutator name
stays as customer_id. That's wrong.

I think I'm reading the code right, and it's consistent with what Patrik
saw. I'm very surprised the CDBI tests haven't picked up on this.

>  I think Tony must have changed the  
> order of events when he factored out the column code.
> 
> incidentally, your lc($column) is likely redundant anyway: $column is  
> an object and stringifies to a lowercased version of its name. As a  
> test, I'd suggest that you either omit the lc($column) or add a  
> simple mutator_name_for method:

Instead of this, my personal suggestion would be to use
Class::DBI::Frozen::301 until there's a stable release.

Cheers, Dave

>    sub mutator_name_for {
>      return lc(shift);
>    }
> 
> and see if that makes any difference.
> 
> best
> 
> will

_______________________________________________
ClassDBI mailing list
ClassDBI@xxxxx.xxxxxxxxxxxxxxxx.xxx
http://lists.digitalcraftsmen.net/mailman/listinfo/classdbi