Declare and initialize variables close to where they are used.
Avoid the C style where all variables have to be defined at the beginning of a block, but rather define and initialize each variable at the point where it is needed.
Bad Example:
public Message GetMessage(int sampleId)
{
var msg = "This is a message";
.
.
if (NeedsMessage)
{
return new Message(msg);
}
}
Good Example:
public Message GetMessage(int sampleId)
{
.
.
if (NeedsMessage)
{
return new Message("This is a message");
}
}
Restrict members and types to the lowest access level by default
To make a more conscious decision on which members to make available to other classes first restrict the scope as much as possible. Then carefully decide what to expose as a public member or type.
Avoid negatives check conditions
To avoid double negative check conditions, look to act on positive conditions by default.
Consider a class that has an error state:
Bad Example: order.HasNoErrors
if (!order.HasNoErrors)
{
// Do something when there are errors
}Good Example: order.HasErrors
if (order.HasErrors)
{
// Do something when there are errors
}Use using statements instead of fully qualified type names
Limit usage of fully qualified type names to prevent name clashing. For example, don’t do this
var list = new System.Collections.Generic.List<string>();
Instead, Do this
using System.Collections.Generic;
var list = new List<string>();
If you Do need to prevent name clashing, use a using directive to assign an alias:
using Label = System.Web.UI.WebControls.Label;
Don’t use “magic” numbers
Don’t use literal values, either numeric or strings, in your code other than to define symbolic constants. For example:
public class Whatever
{
public static reaDonly Color PapayaWhip = new Color(0xFFEFD5);
public const int MaxNumberOfWheels = 18;
}Strings intended for logging or tracing are exempt from this rule. Literals are allowed when their meaning is clear from the context, and not subject to future changes, For example:
mean = (a + b) / 2; // okay
WaitMilliseconds(waitTimeInSeconds * 1000); // clear enoughIf the value of one constant depends on the value of another, Do attempt to make this explicit in the code.
public class SomeSpecialContainer
{
public const int MaxItems = 32;
public const int HighWaterMark = 3 * MaxItems / 4; // at 75%
}Note an enumeration can often be used for certain types of symbolic constants.
Only use var when the type is very obvious
Only use var as the result of a LINQ query, or if the type is very obvious from the same statement and using it would improve readability.
So Don’t
var i = 3; // what type? int? uint? float?
var myfoo = MyFactoryMethod.Create("arg"); // Not obvious what base-class or
// interface to expect. Also difficult
// to refactor if you can't search for
// the class Instead, use var like this.
var q = from order in orders where order.Items > 10 && order.TotalValue > 1000 select order;
var repository = new RepositoryFactory.Get<IOrderRepository>();
var list = new ReaDonlyCollection<string>();In all of three above examples it is clear what type to expect. For a more detailed rationale about the advantages and disadvantages of using var, read Eric Lippert’s Uses and misuses of implicit typing.
JavaScript – Consider “let” over “var”
A “var” declaration is global so it is possible to give rise to “Syntax Error” if uncontrolled. Consider using “let” declaration to limit the scope to the block.
Assign each variable in a separate statement
Don’t use confusing constructs like the one below.
var result = someField = GetSomeMethod();
Favor Object and Collection Initializers over separate statements
Instead of
var startInfo = new ProcessStartInfo("myapp.exe");
startInfo.StandarDoutput = Console.Output;
startInfo.UseShellExecute = true;Use Object Initializers
var startInfo = new ProcessStartInfo("myapp.exe")
{
StandarDoutput = Console.Output,
UseShellExecute = true
};Similarly, instead of
var countries = new List<string>();
countries.Add("Netherlands");
countries.Add("United States");Use collection or dictionary initializers
var countries = new List<string> { "Netherlands", "United States" };Use “left side” comparisons
Consider placing the object to which something is being compared to on the left side.
- This will help prevent accidental setting of a variable or property.
- If you leave out one of the = signs, the compiler will find the error for you.
- It puts the value you are looking for right up front where you can find it instead of buried at the end of your expression. It takes a little time to get used to this format, but then it really gets useful.
Example:
public const int MAX_PAGE_SIZE = 100;
.
.
if (MAX_PAGE_SIZE == pageSize)
{
.
.
}Don’t make explicit comparisons to true or false
It is usually bad style to compare a bool-type expression to true or false. For example:
while (condition == false) // bad
while (condition != true) // also bad
while (((condition == true) == true) == true) // where Do you stop?
while (condition) // OKDon’t change a loop variable inside a for or foreach loop
Updating the loop variable within the loop body is generally considered confusing, even more so if the loop variable is modified in more than one place. Although this rule also applies to foreach loops, an enumerator will typically detect changes to the collection the foreach loop is iteration over.
for (int index = 0; index < 10; ++index)
{
if (some condition)
{
index = 11; // Use 'break' or 'continue' instead.
}
}Avoid nested loops
A method that nests loops is more difficult to understand than one with only a single loop. In fact, in most cases having nested loops can be replaced with a much simpler LINQ query that uses the “from” keyword twice or more to join the data.
Always add a block after keywords such if, else, while, for, foreach and case
Please note that this also avoids possible confusion in statements of the form:
if (b1) if (b2) Foo(); else Bar(); // which 'if' goes with the 'else'
// instead:
if (b1)
{
if (b2)
{
Foo();
}
}
else
{
Bar();
}Always add a default block after the last case in a switch statement
Add a descriptive comment if the default block is supposed to be empty. Moreover, if that block is not supposed to be reached throw an InvaliDoperationException to detect future changes that may fall through the existing cases. This ensures better code, because all paths the code can travel has been thought about.
void Foo(string answer)
{
switch (answer)
{
case "no": Console.WriteLine("You answered with No"); break;
case "yes": Console.WriteLine("You answered with Yes"); break;
default: // Not supposed to end up here.
throw new InvaliDoperationException("Unexpected answer " + answer);
}
}Finish every if-else-if statement with an else-part
For example.
void Foo(string answer)
{
if (answer == "no")
{
Console.WriteLine("You answered with No");
}
else if (answer == "yes")
{
Console.WriteLine("You answered with Yes");
}
else
{
// What should happen when this point is reached? Ignored? If not,
throw new InvaliDoperationException("Unexpected answer " + answer);
}
}Be reluctant with multiple return statements
One entry, one exit is a sound principle and keeps control flow readable.
There are cases where having multiple return statements is more readable but you should generally avoid it. In the case where you have preconditions that must be verified, consider moving that to its own method.
Avoid monolithic methods
If find yourself writing a novel inside a method then you might need to take a step back and refactor it. Consider seven lines the general standard. Doing this makes troubleshooting and readability more reliable.
Don’t use if-else statements instead of a simple (conditional) assignment
Instead of:
if (val > 0) { pos = true; } else { pos = false; }
write:
bool pos = (val > 0); // initialization
Or, instead of:
string result;
if (someString != null)
{ result = someString; }
else
{
result = "Unavailable";
}
return result;write:
return someString ?? "Unavailable";
Encapsulate complex expressions in a method or property
Consider the following example:
if (member.HidesBaseClassMember && (member.NodeType != NodeType.InstanceInitializer))
{
// Do something
}In order to understand what this expression is about, you need to analyze its exact details and all the possible outcomes. Obviously, you could add an explanatory comment on top of it, but it is much better to replace this complex expression with a clearly named method:
if (NonConstructorMemberUsesNewKeyword(member))
{
// Do something
}
}
private bool NonConstructorMemberUsesNewKeyword(Member member)
{
return (member.HidesBaseClassMember && (member.NodeType != NodeType.InstanceInitializer);
}
You still need to understand the expression if you are modifying it, but the calling code is now much easier to read and understand.
Call the most overloaded method from other overloads
This guideline only applies to overloads that are intended for providing optional arguments. Consider for example the following code snippet:
private string someText;
public MyString(string text)
{
this.someText = text;
}
public int IndexOf(string phrase)
{
return IndexOf(phrase, 0, someText.Length);
}
public int IndexOf(string phrase, int startIndex)
{
return IndexOf(phrase, startIndex, someText.Length - startIndex);
}
public virtual int IndexOf(string phrase, int startIndex, int count)
{
return someText.IndexOf(phrase, startIndex, count);
}
The class MyString provides three overloads for the IndexOf method, but two of them simply call the one with the most parameters. Notice that the same rule applies to class constructors; implement the most complete overload and call that one from the other overloads using the this() operator. Also notice that the parameters with the same name should appear in the same position in all overloads.
Important If you also want to allow derived classes to override these methods, define the most complete overload as a protected virtual method that is called by all overloads.
Don’t pass null
String, lists and collections should never be passed as null. If you must pass no data consider setting it to empty.
Only use optional arguments to replace overloads
If the optional parameter is a reference type then it can only have a default value of null. To comply with Section 4.21, you must use overloaded methods instead.
Don’t allow methods and constructors with more than three parameters
If you end up with a method with more than three parameters, use a structure or class for passing multiple arguments such as explained in the Specification design pattern. In general, the fewer the number of parameters, the easier it is to understand the method. Additionally, unit testing a method with many parameters requires many scenarios to test.
Favor returning complex objects over REF or OUT
Using REF or OUT makes code less understandable and might cause people to introduce bugs. Prefer returning complex objects or tuple instead.
Favor retunging struct over tuples
Tuples are very powerful and helpful. However, they become hard to read when it is complex. The members of a tuple are accessed via “item1, item2 …” Consider using a class instead. Consider the following code:
public Tuple<int, long, List<string>, int, int> UploadImages(int productId, List<IFormFile> files)
.
.
Tuple<int, long, List<string>, int, int> countSizeErrors = _orderManager.UploadImages(id, files);
return
countSizeErrors.Item1 >= 1 ? Ok(new
{
AddedImageCount = countSizeErrors.Item1,
Size = countSizeErrors.Item2,
FileErrors = countSizeErrors.Item3,
AnnotatedSamples = countSizeErrors.Item4,
AnnotationsWithDefaultValues = countSizeErrors.Item5
}) :
The variable countSizeErrors contains all the information needed to create a valid JSON return. However, it becomes very hard to troubleshoot. Consider instead using a struct so that the name of the property is shown in intelesense.
* Don’t use parameters as temporary variables
Never use a parameter as a convenient variable for storing temporary state. Even though the type of your temporary variable may be the same, the name usually does not reflect the purpose of the temporary variable.
Always check the result of an “as” operation
If you use “as” to obtain a certain interface reference from an object, always ensure that this operation does not return null. Failure to do so may cause a NullReferenceException at a much later stage if the object did not implement that interface.
Don’t comment out code
Never check-in code that is commented-out. Nobody knows what to do when they encounter a block of commented-out code.
Prefer to use aliases over predefined system types
The code is more readable when you use the alias over the simple system types:
int x = 123;
System.Int32 y = 123;