What result should I return from DBIx::Class::ResultSet::* methods?

238 views Asked by At

I have next code:

#find contact with given email and/or phone
sub contact {
    my( $self, $email, $phone ) =  @_;

    my $user =  $self;
    $user =  $user->search({ email => $email })   if $email;
    $user =  $user->search({ phone => $phone })   if $phone;

    return $user->first; # I think this is wrong
}

It it OK that in my ArtCoin::Schema::ResultSet::User package to return Result?

2

There are 2 answers

0
Alexander Hartmaier On

That depends on what you need. I prefix all methods returning ResultSets with 'search_' and all that return a single object 'find_'.

0
simbabque On

When you build a custom ResultSet, you typically return the new ResultSet object. In the documentation the examples contain an implicit return.

sub active {
  my $self = shift;
  $self->search({ $self->current_source_alias . '.active' => 1 });
}

Because there is no return, it just returns the return value of the last statement. That's the $self->search(...) part.

So your code would be like this:

sub contact {
    my( $self, $email, $phone ) =  @_;

    my $user =  $self;
    $user =  $user->search({ email => $email })   if $email;
    $user =  $user->search({ phone => $phone })   if $phone;

    return $user;
}

Remember that the ResultSet is not actually a result. There has not been a query at that point. If you call ->first on it in your method, it will talk to the database and run your query. After that, you cannot chain additional searches to the ResultSet, because you don't have a ResultSet any more.


The following section is a bit of code-review.

What you did there works quite well, but it's a bit complex. You are chaining up to two additional searches, which translate to more stuff that DBIC needs to do for you in the background. Think about how that method should behave.

Right now, it does this:

  • if there are no arguments $rs->contact(), it will return the same ResultSet as before, without any new search criteria (SELECT * FROM contacts)
  • if there was the email argument $rs->contact($email), it will return a new ResultSet that has an additional search criteria for the email (SELECT * FROM contacts WHERE email='...')
  • if there was the phone argument $rs->contacts( undef, $phone ), it will return a new ResultSet that has an additional search criteria for the phone (SELECT * FROM contacts WHERE phone='...')
  • if there were both email and phone arguments $rs->contacts( $email, $phone ), it will return a new ResultSet that has two additional search criteria (SELECT * FROM contacts WHERE email='...' AND phone='...')

That makes sense, but for the first case. If you don't want any narrowing down, you'd not call that method in the first place, would you?