Hi everyone! There is a lot of information about different C# features. About various life hacks and best practices in this language. I want to tell you about equally useful, but less popular tips for working with this language. 1. Non-async "Task/Task<T>" methods should not return null Returning null from a non-async Task/Task<T> method will cause a NullReferenceException at runtime. This problem can be avoided by returning Task.FromResult<T>(null) instead. Bad example: { ; } Task< > ( ) public object GetFooAsync return null // Noncompliant Good example: { Task.FromResult< >( ); } Task< > ( ) public object GetFooAsync return object null 2. Strings should not be concatenated using '+' in a loop StringBuilder is more efficient than string concatenation, especially when the operator is repeated over and over as in loops. Bad example: str = ; ( i = ; i < arrayOfStrings.Length ; ++i) { str = str + arrayOfStrings[i]; } string "" for int 0 Good example: StringBuilder bld = StringBuilder(); ( i = ; i < arrayOfStrings.Length; ++i) { bld.Append(arrayOfStrings[i]); } str = bld.ToString(); new for int 0 string 3. String offset-based methods should be preferred for finding substrings from offsets Looking for a given substring starting from a specified offset can be achieved by such code: str.Substring(startIndex).IndexOf(char1). This works well, but it creates a new string for each call to the Substring method. When this is done in a loop, a lot of strings are created for nothing, which can lead to performance problems if str is large. To avoid performance problems, string.Substring(startIndex) should not be chained with the following methods: IndexOf IndexOfAny LastIndexOf LastIndexOfAny For each of these methods, another method with an additional parameter is available to specify an offset. Using these methods gives the same result while avoiding the creation of additional String instances. Bad example: str.Substring(StartIndex).IndexOf(char1); // Noncompliant; a new string is going to be created by "Substring" Good example: str.IndexOf(char1, startIndex); 4. Collections should not be passed as arguments to their own methods Passing a collection as an argument to the collection's own method is either an error - some other argument was intended - or simply nonsensical code. Further, because some methods require that the argument remain unmodified during the execution, passing a collection to itself can result in an unexpected behavior. Bad examples: list = List< >(); list.AddRange(list); list.Concat(list); list.Union(list); list.Except(list); list.Intersect(list); list.SequenceEqual(list); = HashSet< >(); .UnionWith( ); .ExceptWith( ); .IntersectWith( ); .IsProperSubsetOf( ); .IsProperSupersetOf( ); .IsSubsetOf( ); .IsSupersetOf( ); .Overlaps( ); .SetEquals( ); .SymmetricExceptWith( ); var new int // Noncompliant // Noncompliant // Noncompliant; always returns list // Noncompliant; always empty // Noncompliant; always list // Noncompliant; always true var set new int set set // Noncompliant; no changes set set // Noncompliant; always empty set set // Noncompliant; no changes set set // Noncompliant; always false set set // Noncompliant; always false set set // Noncompliant; always true set set // Noncompliant; always true set set // Noncompliant; always true set set // Noncompliant; always true set set // Noncompliant; always empty 5. Empty arrays and collections should be returned instead of null Returning null instead of an actual array or collection forces callers of the method to explicitly test for nullity, making them more complex and less readable. Moreover, in many cases, null is used as a synonym for empty. Bad examples: { ; } { ; } Result[] ( ) public GetResults return null // Noncompliant IEnumerable<Result> ( ) public GetResults return null // Noncompliant Good examples: { Result[ ]; } { Enumerable.Empty<Result>(); } Result[] ( ) public GetResults return new 0 IEnumerable<Result> ( ) public GetResults return 6. Results of integer division should not be assigned to floating point variables When division is performed on ints, the result will always be an int. You can assign that result to a double, float or decimal with automatic type conversion, but having started as an int, the result will likely not be what you expect. If the result of int division is assigned to a floating-point variable, precision will have been lost before the assignment. Instead, at least one operand should be cast or promoted to the final type before the operation takes place. Examples: dec = / ; dec = ( ) / ; decimal 3 2 // Noncompliant decimal decimal 3 2 7. Shared resources should not be used for locking Shared resources should not be used for locking as it increases the chance of deadlocks. Any other thread could acquire (or attempt to acquire) the same lock for another unrelated purpose. Instead, a dedicated object instance should be used for each shared resource, to avoid deadlocks or lock contention. The following objects are considered as shared resources: this a Type object a string literal a string instance 8. Threads should not lock on objects with weak identity A thread acquiring a lock on an object that can be accessed across application domain boundaries runs the risk of being blocked by another thread in a different application domain. Objects that can be accessed across application domain boundaries are said to have weak identity. Types with weak identity are: MarshalByRefObject ExecutionEngineException OutOfMemoryException StackOverflowException String MemberInfo ParameterInfo Thread 9. Neither "Thread.Resume" nor "Thread.Suspend" should be used Thread.Suspend and Thread.Resume can give unpredictable results, and both methods have been deprecated. Indeed, if Thread.Suspend is not used very carefully, a thread can be suspended while holding a lock, thus leading to a deadlock. Other safer synchronization mechanisms should be used, such as Monitor, Mutex, and Semaphore. 10. Exceptions should not be explicitly rethrown When rethrowing an exception, you should do it by simply calling throw; and not throw exc;, because the stack trace is reset with the second syntax, making debugging a lot harder. Examples: {} (ExceptionType1 exc) { Console.WriteLine(exc); exc; } (ExceptionType2 exc) { Console.WriteLine(exc); ; } (ExceptionType3 exc) { Exception( , exc); } try catch throw // Noncompliant; stacktrace is reset catch throw // Compliant catch throw new "My custom message" // Compliant; stack trace preserved 11. Exceptions should not be thrown from unexpected methods It is expected that some methods should be called with caution, but others, such as ToString, are expected to "just work". Throwing an exception from such a method is likely to break callers' code unexpectedly. The problem occurs when an exception is thrown from any of the following: Event accessors Object.Equals IEquatable.Equals GetHashCode ToString static constructors IDisposable.Dispose operator ==, !=, <, >, <=, >= implicit cast operators Bad example: { ( .IsNullOrEmpty(Name)) { ArgumentException( ); } } ( ) public override string ToString if string throw new "..." // Noncompliant 12. General exceptions should never be thrown Throwing such general exceptions as Exception, SystemException, ApplicationException, IndexOutOfRangeException, NullReferenceException, OutOfMemoryException and ExecutionEngineException prevents calling methods from handling true, system-generated exceptions differently than application-generated errors. 13. Exceptions should not be thrown in finally blocks Throwing an exception from within a finally block will mask any exception which was previously thrown in the try or catch block, and the masked's exception message and stack trace will be lost. 14. Exception types should be "public" The point of having custom exception types is to convey more information than is available in standard types. But custom exception types must be public for that to work. If a method throws a non-public exception, the best you can do on the caller's side is to catch the closest public base of the class. That is, you lose all that custom information you created the exception type to pass. 15. Destructors should not throw exceptions If Finalize or an override of Finalize throws an exception, and the runtime is not hosted by an application that overrides the default policy, the runtime terminates the process immediately without graceful cleanup (finally blocks and finalizers are not executed). This behavior ensures process integrity if the finalizer cannot free or destroy resources. Bad example: { ~MyClass() { NotImplementedException(); } } class MyClass throw new // Noncompliant 16. "IDisposables" created in a "using" statement should not be returned Typically you want to use using to create a local IDisposable variable; it will trigger disposal of the object when control passes out of the block's scope. The exception to this rule is when your method returns that IDisposable. In that case using disposes of the object before the caller can make use of it, likely causing exceptions at runtime. So you should either remove using or avoid returning the IDisposable. Bad example: { ( fs = File.Create(path)) { bytes = Encoding.UTF8.GetBytes(text); fs.Write(bytes, , bytes.Length); fs; } } FileStream ( ) public WriteToFile path, text string string using var // Noncompliant var 0 return 17. "operator==" should not be overloaded on reference types The use of == to compare to objects is expected to do a reference comparison. That is, it is expected to return true if and only if they are the same object instance. Overloading the operator to do anything else will inevitably lead to the introduction of bugs by callers. On the other hand, overloading it to do exactly that is pointless; that's what == does by default. 18. "Equals(Object)" and "GetHashCode()" should be overridden in pairs There is a contract between Equals(object) and GetHashCode(): If two objects are equal according to the Equals(object) method, then calling GetHashCode() on each of them must yield the same result. If this is not the case, many collections won't handle class instances correctly. In order to comply with the contract, Equals(object) and GetHashCode() should be either both inherited, or both overridden. 19. "GetHashCode" should not reference mutable fields GetHashCode is used to file an object in a Dictionary or Hashtable. If GetHashCode uses non-readonly fields and those fields change after the object is stored, the object immediately becomes mis-filed in the Hashtable. Any subsequent test to see if the object is in the Hashtable will return a false negative. Bad example: age; name; { hash = ; hash += .age.GetHashCode(); hash += .name.GetHashCode(); hash; } public int public string ( ) public override int GetHashCode int 12 this // Noncompliant this // Noncompliant return Good example: DateTime birthday; name; { hash = ; hash += .birthday.GetHashCode(); hash; } public readonly public string ( ) public override int GetHashCode int 12 this return 20. "abstract" classes should not have "public" constructors Since abstract classes can't be instantiated, there's no point in their having public or internal constructors. If there is basic initialization logic that should run when an extending class instance is created, you can by all means put it in a constructor, but make that constructor private or protected. 21. Type inheritance should not be recursive Recursion is acceptable in methods, where you can break out of it. But with class types, you end up with code that will compile but not run if you try to instantiate the class. Bad example: < > { } < > : < < < >>> { } c2 = C2< >(); class C1 T class C2 T C1 C2 C2 T // Noncompliant var new int 22. "new Guid()" should not be used When the syntax new Guid() (i.e. parameterless instantiation) is used, it must be that one of three things is wanted: An empty GUID, in which case Guid.Empty is clearer. A randomly-generated GUID, in which case Guid.NewGuid() should be used. A new GUID with a specific initialization, in which case the initialization parameter is missing. 23. "GC.Collect" should not be called Calling GC.Collect is rarely necessary, and can significantly affect application performance. That's because it triggers a blocking operation that examines for cleanup. Further, you don't have control over when this blocking cleanup will actually run. every object in memory As a general rule, the consequences of calling this method far outweigh the benefits unless perhaps you've just triggered some event that is unique in the run of your program that caused a lot of long-lived objects to die. 24. Sections of code should not be commented out Programmers should not comment out code as it bloats programs and reduces readability. Unused code should be deleted and can be retrieved from source control history if required. 25. "goto" statement should not be used goto is an unstructured control flow statement. It makes code less readable and maintainable. Structured control flow statements such as if, for, while, continue or break should be used instead. P.S. Thanks for reading! More tips coming soon! Special thanks to SonarQube and their rules - https://www.sonarqube.org/
