Logo Questions Linux Laravel Mysql Ubuntu Git Menu
 

Should I suppress CA2204: Literals should be spelled correctly?

I've recently upgraded my project from Visual Studio 2008 to Visual Studio 2010.

By enabling Code Analysis, I'm getting a lot of warnings resulted in rule CA2204: Literals should be spelled correctly.

EDIT:

Let's say I have a method called GetResult(), and in it I want to throw an exception for some reason. I want the exception to say "GetResult() has failed for some reason". This will give me the warning since GetResult isn't a word. I won't get a warning on the method name GetResult(), only if I put it in a string. This is because Get and Result are legal words.

I don't believe that writing GetResult() has failed for some reason is the solution.

EDIT: In MSDN it says:

This rule parses the literal string into words, tokenizing compound words, and checks the spelling of each word/token.

Doesn't that mean that GetResult should be checked as two words: "Get" and "Result"?

Should I suppress CA2204?

like image 410
brickner Avatar asked May 18 '10 14:05

brickner


4 Answers

"Can't initialize MyClass" is not a good message for a developer to introduce into code. It rarely aids in debugging and it only confuses the end user if it's ever displayed.

In general, I would say don't suppress the message because spelling error makes one look much dumber than they really are and that's not a message you want to convey with your app.

In this specific instance, it's really a warning of a poor error message -- either tell the user how to correct it, correct it automatically, or include the actual reason it isn't initializing in your error log.

EDIT: Including OP's edits
Something you can take from this warning is that you shouldn't be revealing code details as part of an error message (primarily because they will be included in the call stack when you log the exception).

GetResult() has failed for some reason
Let's say "some reason" is permissions. The message could read:

You don't have permission to view these results.

There's no need to mention the specific method that failed because the stack trace can be logged automatically.

like image 185
Austin Salonen Avatar answered Nov 12 '22 15:11

Austin Salonen


One way to fix this is to not add the type name into the string directly. Instead pass it as a parameter. For example

var msg = String.Format("Can't initialize {0}", typeof(MyClass).Name);

This has the benefit of both getting around the FxCop rule and being safe for refactoring.

like image 38
JaredPar Avatar answered Nov 12 '22 15:11

JaredPar


May be you should not put class name into literal? What about using or defining exception that can be thrown like this:

throw new CantInitializeClassException(innerException, typeof(MyClass);

My idea was to move more information to more specific exceptions from more general ones. I suggest using the sample above instead of throw new ApplicationException("Cant initialize MyClass");

like image 2
Andrey Avatar answered Nov 12 '22 15:11

Andrey


CA2204 is all about spelling. Unfortunately even in VS2012 it still has an old annoying bug where it fails to tokenize compound words: http://connect.microsoft.com/visualstudio/feedback/details/521030

This caused me to switch off this rule in our code analysis rule set.

like image 1
Knaģis Avatar answered Nov 12 '22 15:11

Knaģis