Sorry to be so slow to respond here. First, thanks Marcos, solid patches. And good work figuring out an easy area to work on to get started.
On Mon, Mar 23, 2015 at 01:47:01PM -0300, Marcos Cardinot wrote: > Hi Lubomir, > > It's not about a dangerous lack of initialization - it's more about code > consistence. So, the question should be why not initialize all members as > we should do? Well, if the code doesn't initialize the member at all, then that's a bug and the patch should point that out. Your first patch (the one to globe.cpp) makes that claim, but that comment is actually incorrect because eventFilter() will be called before mouseClicked() and it ensures that doubleClicked is either true or false... > In addition, most of these are not following the new coding style (which we > discussed in another thread), so if you want to apply them, I could send > new patches fixing it (and join all changes in a single patch if you > like)... What I would like is a series of patches that a) follows the newly clarified CodingStyle b) identifies for each of the changes where the member was previously initialized and how the new code is better (and "easier to read / understand" is a valid reason). BUT, there are cases where I think having the member initialized makes the code harder to understand. Let's take your second patch, the one to updatemanager.cpp. isAutomaticCheck is ALWAYS initialized BEFORE requestReceived() is connected - and that's the only way requestReceived() is ever called. So to me having to different spots that initialize the isAutomaticCheck member actually makes the code harder to understand because when I look at requestReceived() it's no longer immediately where this member was set. Makes sense? Marcos, could you create a new patch series that keeps this in mind and only changes the ones where clarity is improved, and documents the rationale in the commit message? /D _______________________________________________ subsurface mailing list subsurface@subsurface-divelog.org http://lists.subsurface-divelog.org/cgi-bin/mailman/listinfo/subsurface