Code::Blocks

Developer forums (C::B DEVELOPMENT STRICTLY!) => Development => Topic started by: frithjofh on January 16, 2016, 01:28:12 pm

Title: small refactoring in findreplacedlg
Post 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
Title: Re: small refactoring in findreplacedlg
Post by: MortenMacFly on January 17, 2016, 11:09:25 am
To ensure that such things can be applied, please respect our coding guidelines.

In the snippet I would recommend:
Title: Re: small refactoring in findreplacedlg
Post by: frithjofh on January 17, 2016, 03:04:14 pm
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...
Title: Re: small refactoring in findreplacedlg
Post by: frithjofh on January 17, 2016, 03:24:45 pm
the new patch replacing the last

changes made based on the considerations of morten

thx for the input  :)
Title: Re: small refactoring in findreplacedlg
Post by: MortenMacFly on January 21, 2016, 11:36:21 am
the new patch replacing the last
Looks great && applied.
Title: Re: small refactoring in findreplacedlg
Post by: oBFusCATed on January 21, 2016, 09:03:29 pm
Why is this static -> "static const unsigned int max_value"?
Title: Re: small refactoring in findreplacedlg
Post by: MortenMacFly on January 22, 2016, 08:30:16 am
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.
Title: Re: small refactoring in findreplacedlg
Post by: oBFusCATed on January 22, 2016, 08:00:11 pm
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.
Title: Re: small refactoring in findreplacedlg
Post by: MortenMacFly on January 22, 2016, 08:07:17 pm
For this case it probably doesn't matter, but I just wanted to express my concern about this construct.
OK, point taken.