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.
ICloneabledoes not define a spec for
Clone()and hence should not be implemented CS-R1104
ICloneable 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-R1106
The 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 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.
A conditional expression is an expression which contains the code that is to be executed depending on whether a condition is true or false. Having identical expressions for both the true and false branches is likely a mistake and can affect your program's execution path and is therefore recommended that you fix it.
for loop has 3 basic elements:
In this case, the inner
for loop is modifying a variable that belongs to the outer/enclosing
for loop. This can result in an undefined behavior such as infinite loop. It is therefore recommended that you modify the right variable to ensure that your loop terminates as required.
stringwill result in
Casting a generic array of type
object to a
string array will always fail even if all the elements are strings. It is therefore recommended that you use a suitable and correct alternative such as
System.Linq.Select to convert the elements.
lockstatements should be avoided CS-W1076
lock statements allow you to safely access a resource in a concurrent environment. However, there are certain risks associated with it such as properly acquiring and releasing a lock, deadlocks, etc. An empty lock statement acquires the lock and releases it almost immediately and is usually not considered a proper practice and should be replaced with a suitable replacement such as wait handles.
ifstatements should be avoided CS-W1077
If statements allow you to execute code depending on whether the specified condition evaluates to
false. An empty
if statement has no statements in both of its branches and effectively accomplishes nothing. Such statements should be avoided.
Synchronization allows you to safely access resources that may be subject to race conditions. If one of the accessors, getter, for example, utilizes
lock statements, it means that there is room for race condition and that the underlying resource may be concurrently accessible. In such cases, it is possible that there might be a similar race condition for the setter as well. It is therefore recommended that both accessors be mutually synchronized.
DateTime constructor allows you to specify the year, month, day, and time to construct a
DateTime object. However, this constructor throws an
ArgumentOutOfRangeException exception if the specified date is invalid.
The use of addition and subtraction operations can lead to the creation of invalid dates. Instead, it is recommended that you use APIs such as
AddYears() to construct a
DateTime object since these methods handle invalid dates more appropriately.
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.
enumshould be unique CS-W1088
Values in an
enum should always be unique. Having 2 different members of an
enum carry the same value is likely a mistake. Since such a flawed value can affect how your application behaves, it is recommended that you fix it as soon as possible.
ToCharArray()when iterating a string CS-P1004
ToCharArray() returns a
char array whose elements are the individual characters of the
string on which this method is called. However, this call is particularly redundant within a
foreach statement as
foreach allows you to iterate through the types that implement
IEnumerable<T>, such as
string in this case. Therefore, it is recommended that you get rid of this redundant call.
Instantiating a new instance of
StringBuilder requires the initialization of the underneath buffer that holds the
string contents. Creating a new instance in a loop may have an adverse performance impact. Instead, it is recommended that you initialize
StringBuilder outside the loop and reuse it within the loop by clearing it when required.
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
ifstatement with just the condition if all it does is return a
If all your
ifstatement does is return a
bool value in both the then and else blocks, consider replacing the entire
if statement with just the condition.
recordobjects that rely on
Records are structures that are extensively used in serialization and deserialization. However, if your instance of
record takes in parameters that are class'
const fields, consider reusing the same
record instance instead of instantiating a new one.
Clear()to set the items in a
Span<T>to their default values CS-P1023
Fill() method fills
Span<T> with the value specified. However, consider using the
Clear() method instead if you wish to set the items to the default values as it is more performant and designed for this exact purpose.
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.