Review Request 122910: Introduce KMoreTools

[Kevin Funk]

> On March 15, 2015, 10:31 a.m., Dominik Haumann wrote:
> > src/kmoretools/kmoretools.h, line 412
> > <https://git.reviewboard.kde.org/r/122910/diff/1/?file=354447#file354447line412>
> >
> >     Qt API never returns a this pointer when settings some properties.
> >     
> >     While not explicitely stated here, these links may be useful:
> >     - http://qt-project.org/wiki/API-Design-Principles
> >     - http://kate-editor.org/2011/08/28/coding-style-and-api-design/
> >     
> >     For me, this function should be:
> >     void setInitialItemText(const QString & itemText);
> 
> Gregor Mi wrote:
>     First of all thanks for the detailled review.
>     
>     About not returning 'this': I was thinking about the "Fluent Interface" API design principle (https://en.wikipedia.org/wiki/Fluent_interface). But I agree with you: I also did not see it in other code I read so far and I understand the arguments. Can I conclude that there is no QT equivalent and the better practice here is to write a new line for each setter statement?

Yes, don't return `this`.

One notable exception in Qt is QString::arg, which can be chained. However, in general it's not used in Qt, because of increased complexity when debugging such code (just see section "Problems" in the wiki article you've just mentioned).


- Kevin


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/122910/#review77502
-----------------------------------------------------------


On March 11, 2015, 11:06 p.m., Gregor Mi wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/122910/
> -----------------------------------------------------------
> 
> (Updated March 11, 2015, 11:06 p.m.)
> 
> 
> Review request for KDE Frameworks, Emmanuel Pescosta and Jeremy Whiting.
> 
> 
> Repository: knewstuff
> 
> 
> Description
> -------
> 
> Moved from kservice (https://git.reviewboard.kde.org/r/122576/) to here (knewstuff).
> 
> Example usages:
> - dolphin's SpaceInfoToolsMenu: https://git.reviewboard.kde.org/r/122911/
> - kate's project addin: https://git.reviewboard.kde.org/r/122374/
> 
> 
> Diffs
> -----
> 
>   src/CMakeLists.txt 3b9a403eb473e0c3760506b2757c71d603b99775 
>   src/kmoretools/kmoretools.h PRE-CREATION 
>   src/kmoretools/kmoretools.cpp PRE-CREATION 
>   src/kmoretools/kmoretools_p.h PRE-CREATION 
>   src/kmoretools/kmoretoolsconfigdialog_p.h PRE-CREATION 
>   src/kmoretools/kmoretoolsconfigdialog_p.cpp PRE-CREATION 
>   src/kmoretools/ui/kmoretoolsconfigwidget.ui PRE-CREATION 
>   tests/CMakeLists.txt 103c5fc98deaf83288b843cc66a87f2d95ad9dfb 
>   tests/kmoretools/1/a.desktop PRE-CREATION 
>   tests/kmoretools/1/b.desktop PRE-CREATION 
>   tests/kmoretools/1/c.desktop PRE-CREATION 
>   tests/kmoretools/2/kate.desktop PRE-CREATION 
>   tests/kmoretools/2/kate.png PRE-CREATION 
>   tests/kmoretools/2/mynotinstalledapp.desktop PRE-CREATION 
>   tests/kmoretools/2/mynotinstalledapp.png PRE-CREATION 
>   tests/kmoretools/kmoretoolstest.cpp PRE-CREATION 
> 
> Diff: https://git.reviewboard.kde.org/r/122910/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Gregor Mi
> 
>

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