[Dbix-class] DBIx::Class::ResultSet / resolve_prefetch() / Use of uninitialized value in pattern match

[Matt S Trout]

On Fri, Aug 01, 2008 at 09:37:57AM +0100, Chisel Wright wrote:
> On Thu, Jul 31, 2008 at 09:22:56PM +0100, Matt S Trout wrote:
> > I've never seen a case where it is - do you have "normal" code that causes
> > that warning?
> 
> I don't know if this is "normal" but we have this in our codebase:
> 
> Which we use as:
> 
> ---- cut here ----
>     if (defined $attrs->{prefetch}) {
>         $attrs->{group_by} = prefetch_columns(
>             $self,
>             $attrs->{prefetch}
>         );
>     }
> ---- cut here ----
> 
> I can't remember the exact reasoning for this, I think it was "I'm sure
> we can work out what to put in the group-by instead of copying and
> pasting it all over the place".
> 
> If it's wrong, stupid, etc, please let me know why/how.

Well ... I strongly feel that whoever wrote a -sub- -call- that passes
$self as the first argument should be taught what a method is and/or shot.

Plus this stuff smells like it all belongs on a custom resultset class.

But the basic concept seems pretty reasonable - however, you -should- be
providing an alias since without it you could easily get column name
clashes in the SELECT list. Which is why the internals always pass $alias,
which is why I'm not in favour of "just making the warning go away" - your
code will still be potentially buggy, it'll just be less obvious that it
is. But I also don't want to die() on code that does mostly work.

So how about we compromise on "if undef or '', warn loudly" ?

-- 
      Matt S Trout       Need help with your Catalyst or DBIx::Class project?
   Technical Director                    http://www.shadowcat.co.uk/catalyst/
 Shadowcat Systems Ltd.  Want a managed development or deployment platform?
http://chainsawblues.vox.com/            http://www.shadowcat.co.uk/servers/