Review Request: Generate accessors (getters/setters) for C++

[Jonas Jacobi]

> On Jan. 10, 2012, 1:58 p.m., Milian Wolff wrote:
> > languages/cpp/codegen/generateaccessorsdialog.cpp, line 60
> > <http://git.reviewboard.kde.org/r/103613/diff/2/?file=46367#file46367line60>
> >
> >     please change the ui names, don't put the type (txt, chk, rdo, ...) into the name
> >     
> >     stripPrefixes alone is fine enough

What is the reasoning behind this?
If you read unknown code (without having to take a look at the ui file), you get a (simple) idea of the ui, without actually looking at it.
The semantics of for example setChecked() depends on the widget being a checkbox or radiobutton and you know what happens, just by reading the name.
But, if KDevelops policy is to not have such prefixes, i'll remove them, of course.


> On Jan. 10, 2012, 1:58 p.m., Milian Wolff wrote:
> > languages/cpp/codegen/generateaccessorsdialog.h, line 62
> > <http://git.reviewboard.kde.org/r/103613/diff/2/?file=46366#file46366line62>
> >
> >     on_foo_clicked() is not very Qt-like, call it:
> >     
> >     selectAllClicked()
> >     
> >     same for the functions below
> 
> Jonas Jacobi wrote:
>     the naming is for the qt auto signal slot connections, so i don't have to do them manually
> 
> Milian Wolff wrote:
>     qt auto signal slot connections? what is that? can you please show me a link? I still don't like the names though.

http://developer.qt.nokia.com/doc/qt-4.8/designer-using-a-ui-file.html#automatic-connections


> On Jan. 10, 2012, 1:58 p.m., Milian Wolff wrote:
> > languages/cpp/codegen/generateaccessorsdialog.cpp, line 369
> > <http://git.reviewboard.kde.org/r/103613/diff/2/?file=46367#file46367line369>
> >
> >     this looks wrong, why don't you just use ClassDeclaration::isPublicBaseClass ?
> >     
> >     also, this should be done somewhere else already - just grep the code. for signal/slot support e.g. we also need to know whether we have a QObject or not. So please share code, i.e. use stuff from cppduchain helpers or push existing code there and then reuse it
> 
> Jonas Jacobi wrote:
>     I don't use isPublicBaseClass, because i don't have the actual QObject type, i search the hierarchy only by name.
>     If you could tell me, how i can get the type, or at least, where to look for it, i'll gladly change the code.
> 
> Milian Wolff wrote:
>     see e.g. ExpressionVisitor::qObjectPtrType() - please move this code and the related other method(s) to cppduchain/typeutils for reuse

Ah i didn't know i could just create a new instance of a class type, i just assumed object identity is required.
So then i don't move code to typeutils, but just create the qobject type and use isPublicBaseClass.


- Jonas


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


On Jan. 10, 2012, 12:23 p.m., Jonas Jacobi wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/103613/
> -----------------------------------------------------------
> 
> (Updated Jan. 10, 2012, 12:23 p.m.)
> 
> 
> Review request for KDevelop.
> 
> 
> Description
> -------
> 
> Generate accessors (getters/setters) for C++.
> 
> This patch adds the possibility to create accessor methods for class member variables via the gui.
> The dialog offers the following features:
> - shows all members of a selected class in a tree view (with getters/setters as children), if it was opened on a class member that one is pre-selected
> - only shows setters if applicable (non-const member)
> - accessor method definitions can be created inline or separate (in the fitting source file, if applicable)
> - configurable automatic creation of method names with manual override for each accessor method
> - access policy (visibility) setting global and individualy for each accessor method
> - configurable default setter parameter name with manual override for each setter (void setFoo(int myParamterName))
> - selectable getter return type (e.g. Type, Type&, const Type& for a member of type Type)
> - create setters as slots for QObject subclasses
> 
> The code is in a usable state and i would like to get some feedback on the dialog, especially on usability improvements.
> 
> Regards,
> Jonas
> 
> 
> Diffs
> -----
> 
>   languages/cpp/CMakeLists.txt a1fb97c 
>   languages/cpp/codegen/generateaccessorsdialog.h PRE-CREATION 
>   languages/cpp/codegen/generateaccessorsdialog.cpp PRE-CREATION 
>   languages/cpp/codegen/generateaccessorsmodel.h PRE-CREATION 
>   languages/cpp/codegen/generateaccessorsmodel.cpp PRE-CREATION 
>   languages/cpp/codegen/simplerefactoring.h b2187c3 
>   languages/cpp/codegen/simplerefactoring.cpp fd62ccf 
>   languages/cpp/codegen/ui/cppgenerateaccessors.ui PRE-CREATION 
>   languages/cpp/cppduchain/sourcemanipulation.h 6f8a79b 
>   languages/cpp/cppduchain/sourcemanipulation.cpp 3f40eb2 
>   languages/cpp/tests/CMakeLists.txt 40d07b0 
>   pics/mini/private_slot.png PRE-CREATION 
>   pics/mini/protected_slot.png PRE-CREATION 
>   pics/mini/public_slot.png PRE-CREATION 
> 
> Diff: http://git.reviewboard.kde.org/r/103613/diff/diff
> 
> 
> Testing
> -------
> 
> 
> Screenshots
> -----------
> 
> 
>   http://git.reviewboard.kde.org/r/103613/s/413/
> 
>   http://git.reviewboard.kde.org/r/103613/s/412/
> 
> 
> Thanks,
> 
> Jonas Jacobi
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kdevelop-devel/attachments/20120111/2d0ff359/attachment.html>