Review Request: Refactor GroupsModel (from the Contact List) on top of PersonSetModel

George Goldberg grundleborg at googlemail.com
Sun Jul 31 20:29:04 CEST 2011


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/102156/
-----------------------------------------------------------

(Updated July 31, 2011, 6:29 p.m.)


Review request for Telepathy and Martin Klapetek.


Changes
-------

After discussions on IRC with d_ed, I did a bit of qualitative testing and I reckon that he's right about the loops in dataChanged handling being crap. So, I replaced them with signals proxied via the Item class as suggested. This takes away all the frequently-used loops in PersonSetModel.

Also quickly implemented the personsCount/onlinePersonsCount for groups that was missing in the previous patch.


Summary
-------

This patch to telepathy-kde takes the GroupsModel from the contactlist and refactors it into a subclass of PersonSetModel.

Martin: I'm not entirely convinced I understood your algorithm in GroupsModel for when the groups of a Person change, so I might have ended up making it considerably more inefficient in this review request. Please shout if you see anything that should be changed.

While writing this summary, I've just realised that I forgot to implement the [Online]UsersCount in this, so assume that I'm aware of that and will add it in a later version of the patch :)


Diffs (updated)
-----

  src/CMakeLists.txt b0cf53b6bcf6f2b6395ea24f79f50d6d5344d6fd 
  src/KTelepathy/GroupedPersonSetModel PRE-CREATION 
  src/core/contact.h 64bc4de4508f3ccce97e248b979bbc30f23452e2 
  src/core/contact.cpp 246f2afad0ac377aba9e4b94fac5d5d7faa148bf 
  src/core/person.cpp a690a18b926282cb3368cb077cad6371619db61e 
  src/ui/grouped-person-set-model.h PRE-CREATION 
  src/ui/grouped-person-set-model.cpp PRE-CREATION 
  src/ui/grouped-person-set-model_p.h PRE-CREATION 
  src/ui/person-set-model.h 38860ae456a83d698ed19868fbabf4fed6555902 
  src/ui/person-set-model.cpp 6d01c29f7e7173e2b8af99a709fc3a2137d3115d 
  src/ui/person-set-model_p.h PRE-CREATION 

Diff: http://git.reviewboard.kde.org/r/102156/diff


Testing
-------

PersonSetModel still passes modeltest after the refactoring, and the contactlist behaves as expected when adjusted to use this model instead of the GroupsModel directly.


Thanks,

George

-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://mail.kde.org/pipermail/kde-telepathy/attachments/20110731/d6172b53/attachment-0001.htm 


More information about the KDE-Telepathy mailing list