Developer forums (C::B DEVELOPMENT STRICTLY!) > Development

small refactoring in findreplacedlg

(1/2) > >>

frithjofh:
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

MortenMacFly:
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)

frithjofh:
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...

frithjofh:
the new patch replacing the last

changes made based on the considerations of morten

thx for the input  :)

MortenMacFly:

--- Quote from: frithjofh on January 17, 2016, 03:24:45 pm ---the new patch replacing the last

--- End quote ---
Looks great && applied.

Navigation

[0] Message Index

[#] Next page