fix #290987: B# and Cb octave change #5781
Conversation
Resolves: https://musescore.org/en/node/290987. Make sure that B#, B##, Cb, and Cbb describe themselves as being in the correct octave. This is accomplished by calculating the octave based on what the pitch would be if there were no accidental. Thus, there is no need to special-case certain tpcs.
| @@ -763,7 +763,7 @@ int Note::tpc() const | |||
| QString Note::tpcUserName(bool explicitAccidental) const | |||
| { | |||
| QString pitchName = tpc2name(tpc(), NoteSpellingType::STANDARD, NoteCaseType::AUTO, explicitAccidental); | |||
| QString octaveName = QString::number(((epitch() + ottaveCapoFret()) / 12) - 1); | |||
| QString octaveName = QString::number(((epitch() + ottaveCapoFret() - int(tpc2alter(tpc()))) / 12) - 1); | |||
There was a problem hiding this comment.
Shouldn't int(tpc2alter(tpc())) be a static_cast<int>(tpc2alter(tpc())) in order to conform to our coding rules?
There was a problem hiding this comment.
I don't think so. int(...) is used far more throughout the code than static_cast<int>(...).
There was a problem hiding this comment.
OK. The coding rules do say to prefer static_cast<>() over C-style casts, but say nothing about function-style casts.
There was a problem hiding this comment.
int(...) is a C++ style cast, (int)... is C style (just try the former with a plain C89 compiler)
I find static_cast<int>(...) just ugly, confusing and overly verbose
There was a problem hiding this comment.
Yes, int() is a function-style cast that was introduced in C++. I was saying that our coding rules make no mention of function-style casts either way, but do say to use static_cast<>() in lieu of C-style casts.
I prefer static_cast<>() overall because it's easy to spot, it's easy to search for, and it's ugly and cumbersome to type. That last one is important because casts are a code smell and should be discouraged.
There was a problem hiding this comment.
With that 'smell' you have a valid point. See https://rules.sonarsource.com/cpp/RSPEC-871:
C-style casts and functional notation casts are largely functionally equivalent. However, when they do not invoke a converting constructor, C-style casts are capable of performing dangerous conversions between unrelated types and of changing a variable's const-ness. Attempt to do these things with an explicit C++-style cast, and the compiler will catch the error. Use a C-style or functional notation cast, and it cannot.
Additionally, C++-style casts are preferred because they are visually striking. The visual subtlety of a C-style or functional cast may mask that a cast has taken place, but a C++-style cast draws attention to itself, and makes the the programmer's intention explicit.
There was a problem hiding this comment.
I've added that to the coding style now ;-)
Resolves: https://musescore.org/en/node/290987.
Make sure that B#, B##, Cb, and Cbb describe themselves as being in the correct octave. This is accomplished by calculating the octave based on what the pitch would be if there were no accidental. Thus, there is no need to special-case certain tpcs.