Review Request: Fix previous "Detach Tabs" patch

[Francesco Nwokeka]

> 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.

Alphabetical order. If it's a problem I can set it back


> 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.
> >

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


- Francesco


-----------------------------------------------------------
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
> 
>

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