search

alexrothenberg / motion-addressbook

MIT License
89 stars 30 forks source link

Sorting the address book using Apple's recommended method #47

Closed madewulf closed 11 years ago

madewulf commented 11 years ago

Hello,

the current sorting method for the contacts is quite slow currently. It takes 6 seconds for approximatively 1000 contacts on my iPhone 5.

I guess the problem is the creation of one string per user but I did not investigate fully on this side.

I send you this pull request that uses the function provided by Apple to compare contacts. The speed of the sort after this modification went from 6s to 0,06s on my iPhone 5.

I hope that you will like it.

Martin

jmay commented 11 years ago

I like this change, but it needs tests.

The AddressBook::AddrBook#people method (alternative to Person#all) does not do any sorting, because the OS-provided options are so limited. It makes sense to default to using the OS sort order (so you get the same ordering as you see in the Contacts app). Perhaps we could take an optional block to override the default sort?

jmay commented 11 years ago

Martin, did you see the 6-second sort on the device or was that in the simulator? I've never seen it run that slowly in the simulator with 1500-2000 contacts.

On 25 Sep, 2013, at 4:25 AM, Martin De Wulf notifications@github.com wrote:

Hello,

the current sorting method for the contacts is quite slow currently. It takes 6 seconds for approximatively 1000 contacts on my iPhone 5.

I guess the problem is the creation of one string per user but I did not investigate fully on this side.

I send you this pull request that uses the function provided by Apple to compare contacts. The speed of the sort after this modification went from 6s to 0,06s on my iPhone 5.

I hope that you will like it.

Martin

You can merge this Pull Request by running

git pull https://github.com/madewulf/motion-addressbook master Or view, comment on, or merge it at:

https://github.com/alexrothenberg/motion-addressbook/pull/47

Commit Summary

Sorting the address book using Apple's recommended method File Changes

M motion/address_book/ios/person.rb (3) Patch Links:

https://github.com/alexrothenberg/motion-addressbook/pull/47.patch https://github.com/alexrothenberg/motion-addressbook/pull/47.diff

madewulf commented 11 years ago

The problem happened on the device.

jmay commented 11 years ago

Martin, I've submitted a new PR that incorporates your change, with sorting included in the test suite. Would you mind checking that this works in your situation? My fork is at https://github.com/jmay/motion-addressbook

On 26 Sep, 2013, at 1:15 AM, Martin De Wulf notifications@github.com wrote:

The problem happened on the device.

— Reply to this email directly or view it on GitHub.

madewulf commented 11 years ago

Hello,

I just tried, it works without performance problems on my phone. According to me, it is good to go.