Software design principles and MuseScore

I want to share my impressions of getting acquainted with the MuseScore code base.
I touched the code directly when implementing the new feature in the MuseScore, before that, I was only acquainted with libmsсore (MuseScore mobile app) a year and a half.
At the moment, I was suggested to development MuseScore Editor.
And I am sorry for my English.

If it’s simple, there are problems in the MuseScore Editor code base!

This is not criticism, this is expert opinion and superficial review.
My experience is 10 years of professional development and before that 5 years as a hobby

Preamble

First, the brain's ability to process information and operate on entities is very limited. Therefore, one of the important tasks of the developer is the struggle to reduce complexity. By complexity is meant the number of paths and combinations of code execution and the number of entities that you need to keep in mind at one time.

Second, in order not to be left behind (to satisfy needs at the same speed, to keep up with all the others, to remain relevant, etc.), you need to unceasingly change. Those decisions that were once made for certain conditions may be bad for changing conditions, so when conditions change - adding new features, new functionality, new platforms, new formats, lots of new code added, increasing the number of relationships, etc., you need to unceasingly review decisions. And the developer’s task is to sensitively feel this line when the old solution is no longer suitable and a new one needs to be accepted, for example, two classes need to be made from one class, instead of implementation an abstraction should be introduced, modules should be made from a monolith, etc.

There are many sources of information telling how to develop programs. In fact, they all tell how to cope with complexity and stay flexible.

To describe the problems of the MuseScore Editor code, I will use the following sources:

• SOLID design principles, they are intended to make software development more understandable, flexible and maintainable (https://en.wikipedia.org/wiki/SOLID)
• The book, one of my first and most impressing for me - “Steve McConnell - Code Complete” (I recommend rereading it to all developers, both beginners and professionals)
• Anti-Pattern List: https://en.wikipedia.org/wiki/Anti-pattern#Software_engineering

Looking ahead, I’ll say that in the MuseScore Editor code almost all the principles of development are substantially violated.

Single responsibility principle

One of the most important principles of software development, the letter S from SOLID.
That states: every module or class should have responsibility over a single part of the functionality provided by the software, and that responsibility should be entirely encapsulated by the class, module or function. All its services should be narrowly aligned with that responsibility.

> A class should have only one reason to change.
> Robert C. Martin

Antipatterns: “God object”, “Big lump of mud”, “Magic button”, “Fear of creating new classes” and so on.

MuseScore has many God Objects, for example:

• MuseScore - musescore.cpp file - 8000 lines, but moreover, most of the methods of the MuseScore class are implemented in other files (excerptsdialog.cpp, exportaudio.cpp, file.cpp, harmonyedit.cpp, etc., a lot of them). The class does a lot, it performs the function of both the controller and the view, and implements a lot of very different logic.
• ScoreView - scoreview.cpp file - 5165 lines, and also most of the ScoreView methods implemented in other files (dragdrop.cpp, editlyrics.cpp, keyb.cpp, etc., many of them). The class does a lot.
• Score - the score.cpp file - 4850 lines, and also part of the Score methods implemented in other files (edit.cpp, joinMeasure.cpp, layout.cpp, etc., many of them). The class does a lot

Huge objects with vague responsibility - this is one of the main problems of MuseScore.
This also includes problems with the structure of files and folders, for example, in the main project of the mscore application there are fundamentally implementation files of various functions - the user interface, import of other formats, the midi, the audio driver, the mixer, and various services and all this in one heap (although there are some files that are grouped).

There should be a structure of files and folders, grouped by function. Classes must fulfill one responsibility, for example, if MuseScore is considered a controller, then it should not have the implementation of functions itself, it should only delegate the execution to other classes (and even with such a condition it will be difficult)

Open–closed principle

The letter O from SOLID

That states: "software entities (classes, modules, functions, etc.) should be open for extension, but closed for modification"; that is, such an entity can allow its behaviour to be extended without modifying its source code.

MuseScore is designed in such a way that in order to add a new function, it is necessary to change the implementation of current functions.
For example, to add import of a new file format, you can’t just implement some kind of interface, for example IScoreReader, somewhere to register it and that's it. There is no such interface, you need to change the existing code in different places, and it is even possible to change the behavior of the existing code.

Another example. If you need to add a new representation of notes, the current notes layuoting functions are changed in which a large number of conditions and preferences, these functions turn out to be incredibly complex, and changing them can lead to unpredictable consequences (there are 4550 lines of strongly branching code in the layout.cpp file). Instead, there should be an interface, for example, ILayouter implementations which implement the arrangement of notes for different representations, and the general code should be allocated into small utilitarian functions that can be used or not used (but cannot be changed).

The inability to expand functionality without changing the implementation of the current functional, and therefore again and again, changing the implementation of current functions, for adding new ones (exactly new, not the correction of old), is a universal approach in the MuseScore code. There are no (or very few) abstractions and interfaces, everywhere specific implementations and dependence on them.

Liskov substitution principle

The letter L from SOLID

That states: functions that use the base type should be able to use subtypes of the base type without knowing it.

I will skip the analysis of the implementation of this principle; a deeper immersion in the editor's code base is required.

Interface segregation principle

The letter I from SOLID

That states: too large interfaces must be divided into smaller and more specific ones, so that the software entities of small interfaces know only about the methods that they need to work with. As a result, when changing the interface method, program entities that do not use this method should not change.

In the MuseScore code there are practically no interfaces and abstractions, and there are God Objects, such as the MuseScore, ScoreView, Score that are passed to many methods and are available in very many places, we cannot see where it is called, what it depends on, what it requires, and we cannot limit in dependences to reduce a mess and improve complexity control.

Dependency inversion principle

The letter D from SOLID

That states:

• High-level modules should not depend on low-level modules. Both should depend on abstractions (e.g. interfaces).
• Abstractions should not depend on details. Details (concrete implementations) should depend on abstractions.

As was said, in the MuseScore there are practically no abstractions and interfaces.
But there is a dependence of the lower levels on the implementation of the high-level.

For example, a ScoreView is passed to the sequencer, from which it calls some methods. This means, on the one hand, you cannot use this sequencer in a different configuration, with a different view, and on the other hand, the sequencer “knows” about the implementation of the view and what specific methods you need to call, that is, you cannot change the implementation of the view without changing the sequencer.

Another example, in the classes of the domain model OttavaSegment, Spanner, TempoText call method MuseScore::updateInspector, that is, the domain model class “knows" about the presence of an inspector, knows when to call which method. This means that these classes cannot be used in a different configuration, with different behavior of the inspector, or in which the inspector may not be at all (for example, in the mobile application of MuseScore that uses domain model libmscore)

One can also note the widespread use of singleton and global variables (extern).

There should be interfaces, abstractions, and there should be no dependency on a particular implementation. There should be weak binding.