Hi,
i have to make A LOT classes and the styling convention of the normal codeblocks settings are not in accordance with the code styling guidelines of the target, so added some settings to control the style of the generated code.
For this i had to add a settings dialog for the classwizard plugin.
You can find the changes on this branch:
https://github.com/bluehazzard/codeblocks_sf/tree/test/classwizard/addSettingsDlg
possible changes:
* Parameter name of the set function:
- use generic "var" name (default)
- use variable name without prefix
* First letter of set and get function
- upper case (default)
- lower case
* Implementation of set and get functions
- header file (default)
- source file
* function name generation for the set and get functions
- variable name without prefix (default)
- variable name but with first letter upper case (came case) after the set/get
- variable name all lower case
- variable name all upper case
additionally i added in the settings dialog the settings for the file extensions. Right now they are saved from the new class dialog. I added this for convenience...
any comments?
greetings
1. Always use the override keyword when overriding virtual methods
2. Don't commit commented code
3. Indent with 4 spaces
4. Prefer declaring enums for special options instead of using the plane numbers (I'm talking about nameGeneration, firstLetter, implementation)
5. Prefer using simple range based iteration instead of hard to read and easy to make an error while loops. I'm talking about this
while (implementation == 1 && it != m_MemberVars.end() )
This could easily be replaced by an if and for-each loop.
6. Calling Right on empty string I think causes an assert
7. Do not write complex ternary operators, they are hard to read or debug. if-else are better.
8. Use const more often
9. Consider adding a function which can add a single row. The code dealing with code printing is quite hard to read.
I cannot comment if this is useful addition or not because I'm not using this feature.
1. Always use the override keyword when overriding virtual methods
never seen in cb code... Will do!
2. Don't commit commented code
Yea... i will fix this with a rebase
4. Prefer declaring enums for special options instead of using the plane numbers (I'm talking about nameGeneration, firstLetter, implementation)
Isn't is going to clash with the config read functions? (enum <-> int is not that easy anymore with never c++ compiler)
so i have to use static casts, but it seems to be the better solution instead of unchecked ints
const ClassWizard::eFirstLetter firstLetter = static_cast<ClassWizard::eFirstLetter>(cfg->ReadInt(_T("/SetGet/FirstLetter"), 0));
[...]
switch(firstLetter)
{
case ClassWizard::upperCase:
mv.Get << _T("Get"); break;
case ClassWizard::lowerCase:
mv.Get << _T("get"); break;
}
Is this coding style ok?
9. Consider adding a function which can add a single row. The code dealing with code printing is quite hard to read.
I do not understand this sentence.. What do you mean by "add a single row" ?
I will add a other option to make the set function use const ref as parameter.
4. Yes, you need to use casts from int and back to int. Currently I prefer to use constructor like casts, like mytype(myintvar). They make code easier to read. But are not possible to use all the time.
9. The code related to adding code to the editor/file is rather ugly and bad because you have
<< bla << someotherbla << endofline << newlinebla << endofline << more
My suggestion is to do something like appendLine(...); appendLine(...); I think it will make the code easier to read.
@oBFusCATed
I don't want to derail this topic too much, this can lead to philosophical questions like tabs vs. space, but these range-based for loops are one of the best places to use auto imho, so i couldn't resist.
The key point is to write flexible, reusable code and to achieve that you don't "code against the type" but "code against the properties", and for this you don't need the concrete type. For example pointers, in most locations it doesn't matter if it's a raw pointer or a smart pointer (it only matters if you need to create or destroy it), you only need to know what you can do with it, like dereference.
Another example that touches point 1-3 is if you need to store the return value of a method. Something like that:
unsigned int index = getIndex(const Item& item);
Somewhere in your code this method is defined and in that place its return type is defined. Now in this line you again define its return type. You define the same thing in two places, bad if you need to change the definition of the method someday because you need to change all the others places as well. And does it really matter to see in that line that index is an unsigned int? What if it is a size_t? uint64_t? std::vector<Item>::size_type? It is an index, you can increment it, decrement it, compare it against some capacity value, thats what you need to know.
Actually this simple line is one of the reasons why i can't port a quite sized codebase to run in 32 Bit or 64 Bit, the method returns a size_type which changes from 32 Bit to 64 Bit but the unsigned int stays, depending on platform, 32 Bit. With auto i wouldn't need to take care of these places.
Ok, i will look into this as soon as i find time and commit it with your comments fixed.
(const auto& x : container)
is my preferred since some performance problems lately :)
4. Do not prefix enum types with 'e'.
Why?