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

Mon Apr 2 07:49:43 UTC 2012


> On March 30, 2012, 4:05 p.m., Aleix Pol Gonzalez wrote:
> > Please not here I'm just reviewing the screenshots:
> > 
> > - Always start a button's caption in capital letters. "all" -> "All"
> > - The selection maybe would be better in a context menu? I don't like very much to have buttons that change the UI state, because they don't reflect it. I like it better in the "definitions" groupbox, where you specify what to generate.
> > - more or less the same with the "access" groupbox, but I'm unsure how to fix this. You can put the icon there by the icon, no?
> > - The Qt group box is a bit weird too:
> >   - does it appear if the class doesn't inherit QObject?
> >   - "setters as methods" sounds misleading... :)
> > 
> > Hope it all made sense :)
> > 
> > Aleix

> - Always start a button's caption in capital letters. "all" -> "All"
Ok

> - The selection maybe would be better in a context menu? I don't like very much to have buttons that change the UI state, because they don't reflect it. I like it better in the "definitions" groupbox, where you specify what to generate.
The discoverability of selection in a context menu is imho worse and it is probably the most common selection you do, so it should have a prominent place in the gui imho.
Radio buttons don't really fit, because you'd have to add a "none of the above" option, which don't really makes sense to select (what happens then?).
The button solution isn't perfect, but i think it is pretty easy to grasp as the button names are actions and their effect of a click is immediately reflected on the left side.

> - more or less the same with the "access" groupbox, but I'm unsure how to fix this. You can put the icon there by the icon, no?
More or less the same arguments as above. But you're right, icons would be nice.

> - The Qt group box is a bit weird too:
>  - does it appear if the class doesn't inherit QObject?
No

>  - "setters as methods" sounds misleading... :)
You are right, but I couldn't come up with a better naming, do you have one?

Regards,
Jonas


- Jonas


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


On March 30, 2012, 3:28 p.m., Jonas Jacobi wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/103613/
> -----------------------------------------------------------
> 
> (Updated March 30, 2012, 3:28 p.m.)
> 
> 
> Review request for KDevelop.
> 
> 
> Description
> -------
> 
> UPDATE: I also added the possibility to create getters/setters via code completion (can be turned off in the options)
> The settings can now also be edited via the project configuration dialog.
> ---
> 
> 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
> 
> Regards,
> Jonas
> 
> 
> Diffs
> -----
> 
>   languages/cpp/CMakeLists.txt 1577a7b 
>   languages/cpp/codecompletion/context.h a5fdea7 
>   languages/cpp/codecompletion/context.cpp 33dcad1 
>   languages/cpp/codecompletion/generateaccessorshelperitem.h PRE-CREATION 
>   languages/cpp/codecompletion/generateaccessorshelperitem.cpp PRE-CREATION 
>   languages/cpp/codegen/accessors/cppgenerateaccessors.ui PRE-CREATION 
>   languages/cpp/codegen/accessors/generateaccessors.kcfg PRE-CREATION 
>   languages/cpp/codegen/accessors/generateaccessors.kcfgc PRE-CREATION 
>   languages/cpp/codegen/accessors/generateaccessorsconfiguration.h PRE-CREATION 
>   languages/cpp/codegen/accessors/generateaccessorsconfiguration.cpp PRE-CREATION 
>   languages/cpp/codegen/accessors/generateaccessorsdialog.h PRE-CREATION 
>   languages/cpp/codegen/accessors/generateaccessorsdialog.cpp PRE-CREATION 
>   languages/cpp/codegen/accessors/generateaccessorseditor.h PRE-CREATION 
>   languages/cpp/codegen/accessors/generateaccessorseditor.cpp PRE-CREATION 
>   languages/cpp/codegen/accessors/generateaccessorsitem.h PRE-CREATION 
>   languages/cpp/codegen/accessors/generateaccessorsitem.cpp PRE-CREATION 
>   languages/cpp/codegen/accessors/generateaccessorspreferenceskcmodule.h PRE-CREATION 
>   languages/cpp/codegen/accessors/generateaccessorspreferenceskcmodule.cpp PRE-CREATION 
>   languages/cpp/codegen/accessors/generateaccessorssettings.h PRE-CREATION 
>   languages/cpp/codegen/accessors/generateaccessorssettings.cpp PRE-CREATION 
>   languages/cpp/codegen/accessors/generateaccessorssettingsdialog.ui PRE-CREATION 
>   languages/cpp/codegen/simplerefactoring.h b2187c3 
>   languages/cpp/codegen/simplerefactoring.cpp 8ae4e46 
>   languages/cpp/cppduchain/sourcemanipulation.h 6f8a79b 
>   languages/cpp/cppduchain/sourcemanipulation.cpp 3f40eb2 
>   languages/cpp/cppduchain/tests/test_duchain.cpp 02ed83e 
>   languages/cpp/cppduchain/typeutils.h a686c3b 
>   languages/cpp/cppduchain/typeutils.cpp a801d47 
>   languages/cpp/tests/CMakeLists.txt 24d9597 
>   languages/cpp/tests/testaccessorsettings.h PRE-CREATION 
>   languages/cpp/tests/testaccessorsettings.cpp PRE-CREATION 
>   languages/cpp/tests/testcppgenerateaccessors.h PRE-CREATION 
>   languages/cpp/tests/testcppgenerateaccessors.cpp PRE-CREATION 
>   pics/mini/CMakeLists.txt c3316e7 
>   pics/mini/accessor_reader.png PRE-CREATION 
>   pics/mini/accessor_writer.png PRE-CREATION 
> 
> Diff: http://git.reviewboard.kde.org/r/103613/diff/
> 
> 
> Testing
> -------
> 
> 
> Screenshots
> -----------
> 
> 
>   http://git.reviewboard.kde.org/r/103613/s/412/
> 
>   http://git.reviewboard.kde.org/r/103613/s/413/
> 
> 
> Thanks,
> 
> Jonas Jacobi
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kdevelop-devel/attachments/20120402/79e00a58/attachment.html>