Review Request: Activity ResourceInstance class for API review

[Aaron J. Seigo]

> On May 2, 2011, 3:14 p.m., Ivan Čukić wrote:
> > experimental/libkactivities/resourceinstance.h, line 52
> > <http://git.reviewboard.kde.org/r/101273/diff/2/?file=15928#file15928line52>
> >
> >     The thing I don't like about the /empty/ constructor is that the user could thing the class will work if only some props or none are defined.
> 
> Sebastian Trueg wrote:
>     I agree here. IMHO the default constructor should either take all the required parameters or we need some kind of error handling.

i don't think developers will be confused, and we see this pattern throughout Qt. the exception to an "empty" constructor is when the parameters are unchangeable once the object is created. many classes provide convenience constructors in addition where it makes sense; it could be useful here to also have a constructor that accepts a QUrl as well as QObject * parent and nothing else in addition to the "empty" consturctor. 

in any case, the URL (and other parts) are not static within the object; they can be changed (even set to null, one would expect) and as such the "empty" constructor creates an object in the base state and encourages code that is more readable to be written. having a complex constructor does little (or nothing) to clear up which properties are required and which are optional. something to consider: it is possible to return KUrl() from the object, so trying to imply otherwise via the constructor is not particularly accurate. :)

this is one of the issues that was part of Ettrich's "creating good APIs" presentation at the first Akademy :)


- Aaron J.


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


On May 2, 2011, 3:11 p.m., Ivan Čukić wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/101273/
> -----------------------------------------------------------
> 
> (Updated May 2, 2011, 3:11 p.m.)
> 
> 
> Review request for Nepomuk and Plasma.
> 
> 
> Summary
> -------
> 
> Desc of the class is in the class apidocs
> 
> 
> Diffs
> -----
> 
>   experimental/libkactivities/resourceinstance.h PRE-CREATION 
> 
> Diff: http://git.reviewboard.kde.org/r/101273/diff
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Ivan
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://mail.kde.org/pipermail/plasma-devel/attachments/20110504/2e405eef/attachment.htm