mscore/file.cpp and QSettings variables

• Nov 23, 2018 - 14:10
Reported version
3.0
Priority
P3 - Low
Type
Development
Frequency
Few
Severity
S4 - Minor
Reproducibility
Always
Status
closed
Regression
No
Workaround
No
Project
MuseScore

See here:
https://github.com/musescore/MuseScore/blob/100d297e54512c068386431c046…
and here:
https://github.com/musescore/MuseScore/blob/100d297e54512c068386431c046…
Recently changes were made, as part of this commit: https://github.com/musescore/MuseScore/commit/fe50c850976812c9e16bcc5f8…
These changes exposed an issue with the use of QSettings in file.cpp. There is a global variable named "settings" and then there are these local variables now named "set". In the first two links above, the two variables seem to conflict, and the code uses both the local and the global variable. Look a few lines above in the code links to see the local variable named "set" declared inside braces for an if() block.

Comments

• Dec 4, 2018 - 10:16
Status active ⇒ needs info

That commit is mine, and it tried to disentangle local and global variables without changing any of the semantics.
So the behavoir before and after is exactly the same (at least that was the plan, and I don't see it failing here?), the code did use a local and a global variable before too, just in the changed part the names clashed and caused a compiler warning in MSVC.

Do you mean that the global variable should not be used at all? Or only that and no local variable?

Reply

• Dec 4, 2018 - 12:27

I don't believe the global variable should be used here, though I really have no idea why the global exists. In these specific cases, the changes are all local, so the global is moot. I don't see it failing either, but it sure is confusing as to the relationship with the global. What is the global used for?

Reply

• Dec 4, 2018 - 14:04

The global variable is declared here in class MuseScore, mscore/musescore.h:
https://github.com/musescore/MuseScore/blob/d9ccbb168ce08669862402a6354…

musescore.cpp uses it here and in the 3 functions that follow (saveDialogState, restoreDialogState, writeSettings, readSettings):
https://github.com/musescore/MuseScore/blob/d9ccbb168ce08669862402a6354…

But musescore.cpp also declares a local version in all these places:
https://github.com/musescore/MuseScore/blob/d9ccbb168ce08669862402a6354…
https://github.com/musescore/MuseScore/blob/d9ccbb168ce08669862402a6354…
https://github.com/musescore/MuseScore/blob/d9ccbb168ce08669862402a6354…
https://github.com/musescore/MuseScore/blob/d9ccbb168ce08669862402a6354…
https://github.com/musescore/MuseScore/blob/d9ccbb168ce08669862402a6354…
https://github.com/musescore/MuseScore/blob/d9ccbb168ce08669862402a6354…
https://github.com/musescore/MuseScore/blob/d9ccbb168ce08669862402a6354…
https://github.com/musescore/MuseScore/blob/d9ccbb168ce08669862402a6354…

I do not see anywhere that the class MuseScore settings variable is used outside of class MuseScore (after PR 4303 is merged).

Class Preferences also has a QSettings member variable, named _settings. It is used within the class, but not outside it, AFAICT.

Is there more work to be done here? Why all the local variables in musescore.cpp if there is a class member variable? The class variable seems to be useful for restoreDialogState more than anything else. restoreDialogState is only called within file.cpp for file save/open dialog boxes. What other purpose does this class MuseScore member variable serve? Maybe the class variable should be encapsulated by a method, as with the Preferences _settings variable.

Reply

Fix version
3.5.0