wxColour is "better" than wxColor

[Edgar]

In D:AudacitySVNsrcwidgetsMeter.cpp at or near line number 1091:

Code: Select all

      // Setup the colors for the 3 sections of the meter bars
      wxColour green(117, 215, 112);
      wxColour yellow(255, 255, 0);
      wxColour red(255, 0, 0);

Currently these 3 colors are spelled wxColor which works because there is a #define to allow it. Throughout all of Audacity's code this is the only place where wxColor is used (and these a lines are fairly recent additions to the code); every other instance uses the wxColour version. Now that we are using MSVS2013, which does color highlighting of your code:

coloring.png (16.5 KiB) Viewed 2243 times

it makes even more sense to be consistent in the use of "wxColour". It also helps when searching the code for every place in which a color is used (MSVS2013 has only one "Find" window whereas previous versions had to "Find" Windows - previously you could search for both spellings, one in each "Find" window).

patch attached

[steve]

Currently these 3 colors are spelled wxColor which works because there is a #define to allow it.

Personally I'd prefer that "colour" was spelt correctly everywhere. The developer that originally wrote "wx/colour.h" (Julian Smart) is a native speaker of English. I presume that he was persuaded by his American speaking colleagues to provide "wxColor" as a pseudonym, but given that wxWidgets does provide both spellings, then both spellings are perfectly legal. If we were to insist on consistency in which was used in the Audacity code, then as the "official language" of Audacity is "American English", I expect that the American spelling would win.

[Edgar]

given that wxWidgets does provide both spellings, then both spellings are perfectly legal.

Legal, yes but, because one is a #define and the other a class, each variable type is subtly different. Clearly, over the years the Audacity Developers have taken great pains to ensure that the class-type (not #define-type) is maintained throughout. I really believe it should be changed/"fixed" if for no other reason than consistency but, actually to maintain the proper variable type so that IDEs are not confused. Additionally, though trivial, using the #define-type adds extra work to the compilation cycle and adds an extra layer of potential (though unlikely) complication.

When you work on C++ source code do you use a text editor which distinguishes/highlights reserved words, class names, variables etc.? If so, you should be able to visually see the difference between the two spellings.

[steve]

Legal, yes but, because one is a #define and the other a class, each variable type is subtly different

but not so different that it makes any difference to whether the code works.

When you work on C++ source code do you use a text editor which distinguishes/highlights reserved words, class names, variables etc.? If so, you should be able to visually see the difference between the two spellings.

For both wxColor and wxColour, kDevelop shows:

Code: Select all

Macro wxColor 

Preprocessed body:
wxColour 
Body:
wxColour
/usr/include/wx-2.8/wx/colour.h

I can see your point, but I think we would have a hard time convincing anyone that it is an important change (I may be wrong, and often am )

[Edgar]

I can see your point, but I think we would have a hard time convincing anyone that it is an important change (I may be wrong, and often am ;))

It's a trivial change and only someone with serious OCD could find it important. However, it is a symptom of something troubling to me: a lack of attention to details and an unwillingness to do a tiny bit of work because, though demonstrable, the benefit is small. 'nough said, fagedboudit <grin>!

[steve]

though demonstrable

That's the bit that troubles me - is there a demonstrable benefit?

[Edgar]

though demonstrable

That's the bit that troubles me - is there a demonstrable benefit?

I did point out two demonstrable but small - nay, tiny - benefits: IDE editors which do highlighting highlight them differently so, being different, at a glance wxColor looks "wrong" because it is not of type "class"; it takes the compiler longer to compile the code with the #define version.