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