> On July 11, 2011, 11:10 a.m., David Edmundson wrote: > > app/telepathy-chat-ui.cpp, line 115 > > <http://git.reviewboard.kde.org/r/101899/diff/1/?file=26605#file26605line115> > > > > personally I much prefer "foreach ". I think it's a lot easier to read. > > > > and you can just use 'break' instead of this "&& !found". > > > > I wouldn't say your way is any better than what was there before, but > > not any worse either. > > > > Meh. > > > > Francesco Nwokeka wrote: > foreach creates a copy of every item contained in the vector plus the > previous iteration cycled through the whole vector. > We don't need to cycle through the rest of the vector once the chatwindow > has been found. > > I don't like to use "break" in cycles because they don't match a > condition for which we're supposed to exit the cycle. > > I also found this on iterations with for & foreach > http://www.codeproject.com/KB/cs/foreach.aspx
- That link is talking about C#. - foreach here is a Qt addition to the C++ language. Whilst it (theoretically) does copy the list, thanks to implicit sharing that is just incrementing a ref count in the QList object. - using const references saves it copying each item. > On July 11, 2011, 11:10 a.m., David Edmundson wrote: > > app/chat-window.h, line 105 > > <http://git.reviewboard.kde.org/r/101899/diff/1/?file=26602#file26602line105> > > > > I don't understand why you moved this. To me it made more sense next to > > setupChatTabSignals. > > Francesco Nwokeka wrote: > Alphabetical order. If it's a problem I can set it back I tend to group methods by what they do. Or at least try to. - David ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/101899/#review4588 ----------------------------------------------------------- On July 9, 2011, 12:28 p.m., Francesco Nwokeka wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://git.reviewboard.kde.org/r/101899/ > ----------------------------------------------------------- > > (Updated July 9, 2011, 12:28 p.m.) > > > Review request for Telepathy. > > > Summary > ------- > > This patch consists in a tidy-up of the previous patch and the elimination of > unused methods, logic and the tidy up of some methods and functionality. > Added comments to code as well. > The aim of this patch is to keep the code as tidy and simple as possible. > > > Diffs > ----- > > app/chat-tab.h 408bb91 > app/chat-tab.cpp 018868e > app/chat-window.h a46e41b > app/chat-window.cpp 28905fa > app/telepathy-chat-ui.h ad6809f > app/telepathy-chat-ui.cpp 71ac4ee > > Diff: http://git.reviewboard.kde.org/r/101899/diff > > > Testing > ------- > > Created more than one chat forcing the creation in one window, detached, > closed, reopened and all went well. > > > Thanks, > > Francesco > >
_______________________________________________ KDE-Telepathy mailing list [email protected] https://mail.kde.org/mailman/listinfo/kde-telepathy
