This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/103313/ |
On December 3rd, 2011, 1:12 p.m., Ruurd Pels wrote:
kdeui/itemviews/kcategorizedview.cpp (Diff revision 2) void KCategorizedView::rowsAboutToBeRemoved(const QModelIndex &parent,1376 return;Argh. Exit method halfway. I'd prefer reworking the trailing part of the function (refactoring a bit that is) instead of exiting halfway. Should be as easy as moving the rest of the function in the else clause AFAICS on short notice.
Normally I would say yes in short functions. But in this case, there is also another rule that says "do not do your main computation in the else part", and another one that says "express the if then else in positive". There is the main rule that says "make your code readable and maintainable", that I think wins here. By the way, there are two infinite loops with break; to exit from them to do binary search. You are free to create a patch for them.
- Jaime Torres
On December 3rd, 2011, 10:55 a.m., Jaime Torres Amate wrote:
Review request for kdelibs.
By Jaime Torres Amate.
Updated Dec. 3, 2011, 10:55 a.m. Description
Testing
Diffs
|