Replace repetitive code in SettingsObjectWrapper.cpp by functions?

[Dirk Hohndel]

Hi Berthold,

> On Nov 18, 2017, at 12:02 AM, Berthold Stoeger <bstoeger at mail.tuwien.ac.at> wrote:
> 
> SettingsObjectWrapper.cpp is a glue between the C-style pref struct and the 
> Qt-frontend. The setter functions are full of repetitive code (in some cases 
> with subtle differences). In principle they always do:
> - check if difference (return if none)
> - set field in pref struct
> - set field in SettingsObjectWrapper
> - emit signal
> Side note: Unfortunately, in some cases representations in pref and 
> SettingsObjectWrapper differ. :(

Yes - and in most cases I think there are good reasons for that. char * vs
QString for example

> So I wonder if it would be preferred to use helper functions to do that. In 
> the attachment is a proof of concept. It reduces the number of lines by 579, 
> that is a not too shabby 23.9%. I decided to keep the emit external to the 
> functions, because sometimes other actions are taken prior to the emit.

I always like code reduction to remove opportunities for hard to spot bugs

> You might say that the line count is cheating because more information is 
> crammed into a single line. Nevertheless, I think that using helper functions 
> (once matured) should be less error prone. Look for example at setNextCheck():
> It does
>  date.toString() == prefs.update_manager.next_check
> and then
> copy_string(qPrintable(date.toString("dd/MM/yyyy")))
> Both toString() methods will give different things. This kind of discrepancy 
> can be avoided.

Agreed.

> For consistency, I made a few C-strings non-const. And ultimately also the 
> return of system_default_filename(void). If preferred, the latter could be kept 
> const and the const cast away when assigning to the item of the pref struct.

That I'm not so sure about. We try to use these const declarations to prevent
users of these strings from modifying them when they are supposed to be
immutable.

> Note that this is really only a quick proof of concept and I probably 
> introduced numerous bugs. But if you think this is a good idea, I can try to 
> go over each case and check it for plausibility.

The scary part (for me) is that you are using C++ constructs outside of my
comfort zone. I have always pushed back when Tomaz went into what I
consider C++-lala-land.
I think I mostly understand what the template does and how the namespace
is used, but I realize that I'd be uncomfortable editing this code without some
serious Googling and that makes me nervous.

That's not to say that this is wrong. It's to say that I really want to see what
Tomaz or some other C++ programmers think about it.

Linus and I are fundamentally C programmers who use C++ because we
have to in order to be able to use Qt. I'm getting more comfortable with
some of the classes in Qt and their advantages, but I'm still not someone
who feels super comfortable with C++ as a whole.

/D