C#

C#

By DeepSource

Using the logical not operator to invert binary expressions can affect readability CS-R1108
Anti-pattern

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-R1104
Anti-pattern

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.

Implementing IComparable<T> may be particularly useful CS-R1106
Anti-pattern

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.

Missing implementation of System.Exception CS-R1109
Anti-pattern

Inheriting 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.

Conditional branch has the same expression in both the branches CS-W1073
Bug risk

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.

Incorrect variable being modified in nested loops CS-W1074
Bug risk

A for loop has 3 basic elements:

  1. Initial assignment,
  2. Condition, and,
  3. Incrementors and/or decrementors.

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.

Casting a object[] to string[] will result in CastException CS-W1075
Bug risk

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.

Empty lock statements should be avoided CS-W1076
Bug risk
Autofix

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.

Empty if statements should be avoided CS-W1077
Bug risk
Autofix

If statements allow you to execute code depending on whether the specified condition evaluates to true or false. An empty if statement has no statements in both of its branches and effectively accomplishes nothing. Such statements should be avoided.

Getters and setters should be mutually synchronized CS-W1078
Bug risk

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.

Detected the use of arithmetic operation to construct a DateTime object CS-W1080
Bug risk

The 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 AddDays(), AddMonths(), and AddYears() to construct a DateTime object since these methods handle invalid dates more appropriately.

Drop record's body if it does not define any members CS-R1130
Anti-pattern
Autofix

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.

Values in an enum should be unique CS-W1088
Bug risk

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.

Redundant call to ToCharArray() when iterating a string CS-P1004
Performance
Autofix

The 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 or IEnumerable<T>, such as string in this case. Therefore, it is recommended that you get rid of this redundant call.

Consider reusing existing instances of StringBuilder CS-P1020
Performance

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.

Child and parent classes cannot share the same name CS-R1103
Anti-pattern

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

Consider replacing an if statement with just the condition if all it does is return a bool value CS-R1126
Anti-pattern
Autofix

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.

Consider reusing record objects that rely on const parameters CS-P1022
Performance

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.

Consider using Clear() to set the items in a Span<T> to their default values CS-P1023
Performance
Autofix

The 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.

Consider dropping the parameter's name if it is same as the value when instantiating an anonymous object CS-R1127
Anti-pattern
Autofix

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.