Skip to content

Conversation

willdurand
Copy link
Member

I think that this could be interesting since we always return a set of results rather than only one result. Moreover, having a class rather than relying on the array primitive type seems a bit more robust, and makes the API more fluent:

// Before:
$addresses = $geocoder->geocode('foobar');
$address   = empty($addresses) ? null : reset($addresses);

// After:
$address = $geocoder->geocode('foobar')->first();

WDYT?

public function first()
{
if (empty($this->addresses)) {
return null;
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we return null here? IIRC we can't get 0 results as it would lead to a NoResult exception earlier in the execution flow:

try {
    $address = $geocoder->geocode('foobar')->first();
} catch (NoResult $e) {
    $address = null;
}

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes I think we can remove null here. We always have at least one Address in the collection.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To be honest, I would not remove it since the class might be used elsewhere.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I didn't thought it could used somewhere else :S

@willdurand willdurand changed the title Introduce AddressCollection class [RFC] Introduce AddressCollection class Dec 21, 2014
@pyrech
Copy link
Contributor

pyrech commented Dec 21, 2014

I like the idea. What about implementing also the ArrayAccess interface to ease the migration?

@toin0u
Copy link
Member

toin0u commented Dec 21, 2014

It's awesome ! :) It's a great improvement 👍

@willdurand I think we can implement the \Countable interface, I see the count method :)
@pyrech IMHO the \IteratorAggregate is enough but yes the ArrayAccess is very handy :)

@willdurand
Copy link
Member Author

cool!

👍 for Countable
👎 for ArrayAccess

@willdurand willdurand force-pushed the collection branch 2 times, most recently from fda6311 to 37c9f49 Compare December 22, 2014 10:51
willdurand added a commit that referenced this pull request Dec 22, 2014
[RFC] Introduce AddressCollection class
@willdurand willdurand merged commit d7363b2 into master Dec 22, 2014
@willdurand willdurand deleted the collection branch December 22, 2014 10:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants