Code::Blocks Forums
Developer forums (C::B DEVELOPMENT STRICTLY!) => Development => Topic started by: frithjofh on January 16, 2016, 01:28:12 pm
-
hi everybody,
just some minor refactoring in this class:
- change some ints to size_t
- added some new lines for clarity
- added some conts in variable declarations
- changed IsEmpty() to empty() member function on wxStrings
- refactrored function FindReplaceDlg::FillComboWithLastValues:
- removed loop and replaced it with member function which adds a wxArrayString
- removed check for empty strings as it is not necessary. when the values get written into config
this check is already performed
- refactored function FindReplaceDlg::SaveComboValues:
- changed order of actions. instead of first adding the entries and then checking for the currently
selected entry an d adding it, and removing it... , now it first gets the current selected value and
adds it, and then goes through adding all other entries, skipping empties and doubles
as always: just refactoring. I sometimes use my low quality free time while traveling or sick, to just randomly jump into some file of c::b and read it, checking for strange things, or things that are hard to read at first glance, or that could hide some error, or some superfluous code...
please don't scold me for it. this is my free time ;) and I can't spend it for things that would require a better programmer. making this small clean-ups eventually contributes a little bit every time to make c::b even better than it is ;D
regards
frithjof
-
To ensure that such things can be applied, please respect our coding guidelines.
In the snippet I would recommend:
- Don't use STL methods on wx containers. The reason is that is is confusing if used in a mixed fashion, although it will do the same thing. We've started with wx (because there were no STL methods) so we would like to stick with it.
- after if, for (...) statements insert a space before the opening bracket.
- Things like:
const unsigned int item_count = std::min(combo->GetCount(), 10u);
...should be commented. Because its is not clear in the first place why you picked "10". - Usually, when calling a method in a bracket construct (so its not a variable) you should place spaces around it. So this:
if (!dir.IsEmpty())
...becomes that:
if ( !dir.IsEmpty() )
...but this is OK:
if (!bool_var_no_method_call)
-
ok, I will do the according changes...
just one question: the value of maximally 10 entries written from the wxComboBox into the config was already there before my changes. there was no comment as to this value there neither. I will just add some generic comment then, because I don't know the exact reason why it was put there by the original author. I suppose it was meant to limit the amount of data in the config, but I am not sure...
-
the new patch replacing the last
changes made based on the considerations of morten
thx for the input :)
-
the new patch replacing the last
Looks great && applied.
-
Why is this static -> "static const unsigned int max_value"?
-
Why is this static -> "static const unsigned int max_value"?
Because it must not be instantiated every time you can this function. A member var would also have been fine, but its used only there, locally and not by other methods.
-
I hope you know that in c++11 and on linux. Every local static variable is protected by a mutex/atomic.
I don't know if this is the case for static const variables, but it should be kept in mind when using static.
Especially when the variable is not a basic type.
For this case it probably doesn't matter, but I just wanted to express my concern about this construct.
-
For this case it probably doesn't matter, but I just wanted to express my concern about this construct.
OK, point taken.