Enum values are always accessed using the enum's name. Therefore, it does not make any sense to prefix the enum's values with the enum's name. Doing so is repetitive and adds noise to the code.
Records are majorly used in serialization and deserialization. Either they take in some parameters or define members that participate in this process. However, a record
that neither declares any parameters nor any members achieves nothing and is likely a bad design pattern. It is recommended that you rethink if a record
is really the right structure for your use case.
Min()
/Max()
on SortedSet<T>
CS-P1025The methods Min()
and Max()
come from the Enumerable
interface defined in System.Linq
.
While they may be appropriate for other structures and scenarios, they are not performant for SortedSet<T>
.
Consider using Sorted<T>
's own properties Min
and Max
as they're efficient and tailored for this very purpose.
Culture
-related information when parsing Date
and Time
-s CS-W1092Failing to pass Culture
-related information when parsing Date
and Time
-s prompts the runtime to use the host system's information.
If your application runs on systems that are spread across different regions with different locales, this can cause problems with the formatting.
One such example is DD/MM and MM/DD. To avoid such pitfalls, always pass in the Culture
-related information.
readonly
CS-R1137The readonly
modifier can be applied to fields that are not initialized anywhere in a class and if initialized, it is either initialized inline or in the constructor.
This modifier prevents you from rewriting/overwriting its values and may even allow the runtime to perform additional optimizations.
Consider using this modifier when and where possible.
Length
-like property is compared against values which always evaluate to the same result CS-W1090Length
-like properties are usually used to "count" the number of elements.
For example, in the context of an array, Length
tells us the number of elements in the array.
Comparing this property against 0
using the <
or >=
operators is meaningless and will always evaluate to false
and true
respectively.
This comparison is likely a mistake and should be rewritten meaningfully.
DateTime.Now
that relies on system-specific information CS-W1091DateTime.Now
refers to the date and time that is local and specific to the computer on which the program is running.
Because you may either misinterpret this information or miss out on any additional information, it is recommended that you use DateTime.UtcNow
.
This ensures that your program, irrespective of which computer it runs on, utilizes information that is kept consistent.
IEnumerable.Skip()
takes in number of elements to skip rather than the index CS-W1093The Skip()
method takes in an int
parameter that denotes the number of elements to skip rather than the index of the element to skip.
Passing in 0
to this method is the same as saying "not to skip any element".
The correct syntax instead is Skip(1)
. Consider making the appropriate changes to ensure your program does not rely on flawed logic.
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.
lock
on local variables is error-prone CS-W1094Trying to acquire a lock on a local variable is error-prone as different threads may end up locking on different instances of the same variable. Instead, you should be locking on a class field that is preferably designated as private
and readonly
, i.e. a dedicated object. Microsoft guidelines explicitly state that you should avoid locking on:
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.
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.
A 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.
object[]
to string[]
will result in CastException
CS-W1075Casting 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.
lock
statements should be avoided CS-W1076lock
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.
if
statements should be avoided CS-W1077If 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.
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
object CS-W1080The 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.