Rule | Description | Example | KPI |
---|---|---|---|
CatchBlocksShouldRethrow | Any catch block should rethrow or throw and not eat exceptions. | Cause: A Catch block that does don't throw any exception is not recommended. Non-complaint Code: try Complaint Code: try | Analyzability |
CompilerWarningLevelIsSetTooLow | Build with the highest warning level | Cause: Warnings are potential problems with your code. Turning on the warnings at the highest level will highlight these problems, allowing you to fix them //early, rather than let them become potential bugs at a later date. Complaint Code: You can change the warning level for a project by right-clicking on it, going to its properties page and then setting warning level to highest ie.4 on its Build tab. | Understandability |
DisposeFieldsProperly | This class has a disposable field and is not disposing it. | Non-Complaint Code:internal class D : System.IDisposable Compliant code: class Class1 : System.IDisposable | Maintainability |
DoNotHideInheritedMembers | On hiding an inherited member, the derived version of the member replaces the base class version | Cause: Using a new keyword for a base class member in the derived class, hides the base class member. Ensure the required functionality and look out for compiler warnings. To simply put, if a method is not hiding the base method, it should be overriding it. Complaint code: public class B Non-compliant code: public class B | Understandability |
DoNotPrefixLocalCallsWithThisKeyword | A call to an instance member of the local class or a base class is prefixed with ‘this’. | Non-compliant code:class Employee Compliant code: class Employee | Understandability |
FieldsMustBePrivate | Make the field private and add a property to expose the field outside of the class | Cause: A violation of this rule occurs whenever a field in a class is given non-private access. For maintainability reasons, //properties should always be used as the mechanism for exposing fields outside of a class, and fields should always be //declared with private access. This allows the internal implementation of the property to change over time without changing //the interface of the class. Issue occurence: Fields located within C# structs are allowed to have any access level. Solution: To fix a violation of this rule, make the field private, and add a property to expose the field outside of the class. | Maintainability |
HardCodedStringsAndMagicNumbers | Hard coded values should be moved to resources for better maintainability | Cause: For better maintainability of code it is recommended to have all strings in one place and you only need to change it there to update the whole program. Non-complaint code: public void MyMethod() Compliant code: public void MyMethod() | Maintainability |
MakeLocalVariableConstant | If a variable is assigned a constant value and never changed, it can be made ‘const’ | Cause: The variable is assigned a constant value and never changed, it can be made a const Non-compliant code: class Class1 Compliant code: class Class1 | Maintainability |
RedundantFieldAssignment | Consider not assigning the default value to a field for performance optimisation | Cause: It’s recommended not to assign the default value to a field as a performance optimization. Non-complaint code: class SampleClass Complaint code: class SampleClass | Efficiency |
ThrowDoesNothing | If an exception is caught and then thrown again the original stack trace will be lost. It is best to throw an exception without using any parameters | Cause: Throwing the same exception as passed to the ‘catch’ block lose the original stack trace and will make debugging this exception a lot more difficult. The correct way to rethrow an exception without changing it is by using ‘throw’ without any parameter. Non-complaint code: try { } Complaint code: You will have two choices, you can rethrow the original exception as-is: try { } | Analyzability |
UseConfigureAwaitFalseOnAwaitedTask | Consider using ConfigureAwait(false) on the awaited task | Non-complaint code:public async Task TestAsync() Complaint code: public async Task TestAsync() | Robustness |
UseOfRegexIsMatchMightBeImproved | Instantiating the Regex object multiple times might be bad for performance. Consider using the static IsMatch method from Regex class and/or compile the regex | Cause: When IsMatch() method is called multiple times, you should be using the static Regex.IsMatch method, as that can be cached and won’t be interpreted //every time you call the method, it will give you better performance. Non-complaint code: public static bool TestRegex(string parameterValue) Complaint code: public static bool TestRegex(string parameterValue) | Efficiency |
UseStaticMethod | If the method is not referencing any instance variable and if you are not creating a virtual/abstract/new or partial method and if it is not an overridden method, your instance method can be changed to a static method. | Non-complaint code:public class Program Complaint code: public class Program | Robustness |
UseSwitch | Multiple ‘if’ and ‘else if’ statements can be replaced with a ‘switch’ | Cause In a switch statement, all elements get the same access time, compared to a list of if-else’s where the last element takes much more time to reach as //it has to evaluate every previous condition first. Thus the compiler is better in optimizing a switch-statement than an if-statement. Non-complaint code: public void DoThis(string s) Complaint code: public void DoThis(string s) | Efficiency |
LogErrorInCatch | The log level inside Catch block should be Error | Cause A log level less than an Error is not recommended within a Catch Block especially when there is an exception. Non-complaint code: try Non-complaint code: try | Maintainability |