Using the logical not operator !
to invert the result of a binary expression's result can affect readability as it requires that the reader first comprehend the binary expression and then mentally invert the result. This can interrupt the natural flow of reading the code, thereby affecting readability. It is recommended that you refactor this expression.
ICloneable
does not define a spec for Clone()
and hence should not be implemented CS-R1104ICloneable
allows you to define methods that help in cloning the instances of your class. However, the specification does not define whether this clone operation is a shallow clone or a deep clone. If it is a deep clone, it may end up recursively referencing other objects in the object graph. Moreover, if a class implements ICloneable
, there may be a need for all its subtypes to implement it too. It is therefore recommended that you define your own method that aids in cloning.
IComparable<T>
may be particularly useful CS-R1106The specified class has a method whose signature resembles IComparable<T>::CompareTo(T? other)
but does not implement IComparable<T>
. If your method indeed performs a comparison between 2 objects of the same type, it may be particularly useful to implement the IComparable<T>
interface which is defined exactly for purposes like these.
System.Exception
CS-R1109Inheriting System.Exception
class allows you to define your own custom exceptions. This is particularly useful for scenarios where you believe that the exceptions supplied in the standard library are not suitable for your use cases. The norm is that such user-defined exception's name end with the word Exception
such as EmployeeListNotFoundException
. However, in this case, such a class was found to not inherit System.Exception
. Either consider inheriting it, or, renaming your class to avoid confusion.
Record is an entity that is primarily (but not limited to) used for supporting immutable data models and is commonly used in serialization and deserialization. Consider dropping the record's body if it takes in one or more parameters and does not define any members.
Using the same name for both the child class and the parent class can cause confusion. It is recommended that the names be as unique as possible
if
statement with just the condition if all it does is return a bool
value CS-R1126If all your if
statement does is return a bool
value in both the then and else blocks, consider replacing the entire if
statement with just the condition.
Initializers let you specify the parameter name explicitly when instantiating an object (both named and anonymous objects). However, if you're instantiating an anonymous object and the parameter's name is same as the value, consider dropping the name altogether as it is redundant.
else
statement is redundant CS-R1129The else
clause is used to define what happens in case the if
condition evaluates to false. Any empty else
clause is redundant and can be dropped safely. Either add a user comment inside explaining why the said clause is empty or drop it altogether.
default
label without a comment is redundant CS-R1022The default
case in a switch
is executed when none of the provided cases match. An empty default
case without a user comment is redundant. Either add a user comment to indicate to the reader and the analyzer that all the possible cases have been covered, or, consider throwing a NotImplementedException
to indicate that some cases are yet to be handled.
bool?
CS-R1118While the null coalescing operator may come in handy when evaluating the bool?
types, it is recommended that you stick to the traditional equality operators and explicit boolean values — an approach that is more readable and easier to comprehend.
not null
instead of an empty recursive pattern when checking for nullness CS-R1120One way to check for nullness using pattern matching is to use the empty recursive pattern syntax {}
. However, consider using not null
instead as it is more readable and easier to comprehend as it accurately conveys what it achieves.
public
class per namespace CS-R1035Having more than 1 public class per namespace increases the complexity and affects the overall code readability and navigation. It is therefore recommended that you have a single public
class per namespace
.
NullReferenceException
CS-R1009Explicit trapping of NullReferenceException
is usually considered a bad practice. It usually means that your application has come across a null
reference in an unexpected manner which you're trying to suppress by explicitly trapping through a catch
block rather than finding the root cause. Since this was unexpected, it is probably not safe for your application to continue with the execution.
var
is redundant when combined with out
and a discard pattern CS-R1067The out
keyword is used to indicate that the parameter is initialized by the method to which it is being passed. But, when a discard pattern (_
) is used with out
, it indicates that the variable is unused and is only a placeholder.
Therefore, in such cases, the keyword var
becomes redundant and can be dropped.
Exception
is unused CS-R1084Exception variable declared in catch
should be used. If you do not wish to use the trapped Exception
's details, consider omitting the variable from the catch
declaration.
The result of the specified bitwise operation is same as the operand supplied to the bitwise operator. It is possible that you meant to specify a different operand. It is recommended that you review the said bitwise operation to ensure it does what you need.
namespace
per file CS-R1034Namespaces are used to organize classes and control the scope of class and method names in larger projects. Having more than 1 namespace per file increases the complexity and affects the overall code readability and navigation. It is therefore recommended that you have a single namespace per file.
The ternary operator ?:
evaluates a boolean expression and returns the result of one of the two expressions, depending on whether the expression evaluates to true or false. While the ternary operator may be particularly useful in avoiding simple if
statements, it can, however, affect the readability when nested. Therefore, it is recommended that you avoid nesting such operators.
If your lambda's body has a single statement, consider refactoring it to move away from block syntax to expression body. Doing so makes your code easier to read.