Access ConfigObjects by predefined constants instead of plain strings

[uklotzde]

...just migrated some config keys as a starting point and prerequisite for #4218.

An attempt to tame the chaotic string-based access to configuration settings. Please give feed back what you think about it. The _prefs.h file serves a similar purpose as the _decl.h file. Instead of static class members namespaces are used. You don't need the whole class definition for this purpose, only some shared constants.

[coveralls]

Pull Request Test Coverage Report for Build 1152289915

• 7 of 22 (31.82%) changed or added relevant lines in 4 files are covered.
• 1 unchanged line in 1 file lost coverage.
• Overall coverage increased (+0.008%) to 25.969%

---

Changes Missing Coverage	Covered Lines	Changed/Added Lines	%
src/preferences/upgrade.cpp	0	1	0.0%
src/library/trackcollectionmanager.cpp	2	4	50.0%
src/library/library.cpp	0	12	0.0%

Files with Coverage Reduction	New Missed Lines	%
src/library/library.cpp	1	0%

Totals
Change from base Build 1152021735:	0.008%
Covered Lines:	20010
Relevant Lines:	77053

---

💛 - Coveralls

[daschuer]

What is the advantage of using namespaces and the extern keyword from ANSI-C legacy. For my understanding it is a big benefit to use classes, that the the compiler is able to do some sanity checks.

[uklotzde]

extern is not legacy. It is needed to create a single definition of the constant. Otherwise you get linker errors due to multiple definitions. If you place the definition in the header file you also end up with multiple definitions, one for each inclusion of the header.

Why should we enclose constants in classes instead of namespaces?? The heavy-weight Library class is not needed if you just need to access the config object.

[daschuer]

The issue with "extern" comes up when you have typos in the cpp file:

In this case you get the linker error like that, without a hint where the typo is:

undefined reference to `mixxx::library::prefs::kLegacyDirectoryConfigKey'

Including the library_prefs.h in the library_prefs.cpp is redundant in you current implementation.

If you do the same with a class, you get a compiler way earlier error with a detailed location:

library_prefs.cpp:27:5: error: ‘const ConfigKey mixxx::library::prefs::kLegacyDirectoryConfigKey1’ is not a static data member of ‘class mixxx::library::prefs’

I have just noticed that you can achieve an early compiler error as well, if you remove the inner namespace and use it as prefix

library_prefs.cpp:8:24: error: ‘mixxx::library::prefs::kLegacyDirectoryConfigKey1’ should have been declared inside ‘mixxx::library::prefs’

That solves the issue for me.

[uklotzde]

I have removed the nested namespaces in the .cpp file. But this requires a comment that explains the reason why this is needed.

We should strive to find a fool-proof solution that could be applied to all use cases in a straight-forward manner.

[daschuer]

The current solution with the extra comment is already good IMHO.
Merge?

[uklotzde]

I wonder why only the Windows build failed? https://github.com/mixxxdj/mixxx/pull/4223/checks?check_run_id=3386161822

Should work now, let's see.

We could use this as the starting point and keep an eye on usability issues that might occur while migrating more ConfigObject access code.

[daschuer]

OK, LGTM. Thank you.