Replace repetitive code in SettingsObjectWrapper.cpp by functions?

Berthold Stoeger bstoeger at mail.tuwien.ac.at
Sat Nov 18 00:02:56 PST 2017


Dear all,

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

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.

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.

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.

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.

Berthold

-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-Replace-repetitive-code-in-SettingsObjectWrapper.cpp.patch
Type: text/x-patch
Size: 48740 bytes
Desc: not available
URL: <http://lists.subsurface-divelog.org/pipermail/subsurface/attachments/20171118/24e1a443/attachment-0001.bin>


More information about the subsurface mailing list