Review Request 113197: KMessageBox: Factorize code between regular and *WId functions

[Commit Hook]

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


This review has been submitted with commit d16abdc34489e19d67145b58a3ece1c8f4c42e96 by Aurélien Gâteau to branch frameworks.

- Commit Hook


On Oct. 11, 2013, 8:56 a.m., Aurélien Gâteau wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/113197/
> -----------------------------------------------------------
> 
> (Updated Oct. 11, 2013, 8:56 a.m.)
> 
> 
> Review request for KDE Frameworks.
> 
> 
> Repository: kdelibs
> 
> 
> Description
> -------
> 
> Factorize code between regular and *WId functions, reducing the file size by 12%.
> 
> This patch adds another level of indirection. For example the sorry() function is changed from:
> 
>     void sorry(QWidget *parent, ...args)
>     {
>         QDialog *dialog = new QDialog(parent);
>         /* fill dialog */
>     }
> 
> to:
> 
>     static void sorryInternal(QDialog *dialog, ...args)
>     {
>         /* fill dialog */
>     }
> 
>     void sorry(QWidget *parent, ...args)
>     {
>         sorryInternal(new QDialog(parent), ...args);
>     }
> 
> This makes it possible to turn the sorryWId() function into a forward function rather than a slightly-different copy of the original sorry() function:
> 
>     void sorryWId(WId parent_id, ...args)
>     {
>         sorryInternal(createWIdDialog(parent_id), ...args);
>     }
> 
> createWIdDialog() is a helper function to create a dialog which is a child of a window belonging to another process.
> 
> Note: I kept most of the code to the place where it originally was in the file. This keeps the diff small, but readability would be improved by grouping together similar functions. Let me know if you think it is worth doing so.
> 
> 
> Diffs
> -----
> 
>   tier1/kwidgetsaddons/src/kmessagebox.cpp 0cfa491 
> 
> Diff: http://git.reviewboard.kde.org/r/113197/diff/
> 
> 
> Testing
> -------
> 
> Builds, kmessageboxwidtest.cpp runs correctly. No other tests are available though.
> 
> 
> Thanks,
> 
> Aurélien Gâteau
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-frameworks-devel/attachments/20131014/d2c38475/attachment.html>