-
Notifications
You must be signed in to change notification settings - Fork 2
Coding Conventions
Hector edited this page Jun 23, 2020
·
8 revisions
We typically follow these conventions when coding in most cases. We may request changes to your pull request based on the below. Please note that this doesn't always mean your code is "wrong", but it may be inconsistent relative to the conventions we've adopted. Where possible, we'll explain why we used certain conventions. Also note that existing code may not be perfect. These conventions are a dynamically growing thing and we don't always have the resources to refactor old code to meet new standards.
Convention | ❌ | ✅ | Explanation |
---|---|---|---|
Use Explicit Types (not var ) |
var str = ""; |
string str = ""; |
We always prefer explicit types vs var . We feel the code is easier to read when the type is right there. |
Use Interface Types | List<MyClass> = ... |
IList<IMyInterface> = ... |
Unless there's good reason to do so, avoid using raw classes. Use interfaces instead. This makes the code easily separable by the interface separation principle. So if you have a class A and a class B which uses A , rather than reference A directly from B , the recommendation is to create an interface IA , exposing only the public methods of A and then make B depend on IA . |
Variable Naming | int i = 0 |
int index = 0 |
As far as possible, variables should be named so that the meaning of the variable can be inferred from the variable name. You can always start coding with variables like i and j and then when it's time to commit your code, you can use your IDE to rename the variables to something more descriptive e.g. rowIndex and columnIndex . The variable names should tell a story. They should be descriptive enough that you rarely ever need to use comments. |
Boolean Naming | bool found = true |
bool isFound = true |
Functions, properties or variables that are of a boolean type can usually be named with an "is" or "are" prefix. When this is possible, it is recommended that functions, variables and properties are all named this way. Booleans that assert some condition about a single object usually can start with "is" while those that assert a condition over multiple objects can start with "are". Note: for properties/functions the first letter will be uppercase. For example, suppose we have a variable int count . If we want to store the result of whether this count is positive, we can use the name isCountPositive . For an example of a plural condition, consider an array int[] values . Now suppose we want to write a function that returns true whenever all the values in the array are positive. Following our convention, we'd choose a name such as AreAllValuesPositive . Note: for the "are" case, to avoid ambiguity, we should also specify which kind of quantification we're doing: "all" vs "some". |
Use Local Variables to store Interim Calculations | double totalVolume = length1*breadth1*height1 + length2*breadth2*height2); |
double volume1 = length1 * breadth1 * height1; double volume2 = length2 * breadth2 * height2; double totalVolume = volume1 + volume2;
|
This convention could also have been called "avoid using lengthy expressions". It's best demonstrated with an example. Suppose you have this code: PrintTotalVolume(length1*breadth1*height1 + length2*breadth2*height2) . Instead, we would favour four lines of code: double volume1 = length1*breadth1*height1; then double volume2 = length2*breadth2*height2 , then double totalVolume = volume1 + volume2 and finally PrintTotalVolume(totalVolume) . |
Clean Code, not Cluttered Comments |
// calculate minutes per hour int a = 60 * 60;
|
int minutesPerHour = 60 * 60 |
If you are following other best practices such as good variable naming and using local variables to store interim calculations, then your code should tell a story as it is written. If you find yourself beginning to write a comment to explain a bit of complex code, the first rule of thumb is: can the code itself be broken down, simplified, and renamed in such a way that the comment is unnecessary? If so, you are always advised to make the code change rather than add comment clutter on top of complex code. There are exceptions to this, but they are rare. For example you might need to do some weird trick to avoid a bug in an API over which you have no control. In that case, a comment is absolutely justified. |
Generic Type Parameter Names | interface MyTransformer<T, U> |
interface MyTransformer<TInput, TOutput> |
When there is a single generic type parameter, with an obvious meaning from the context, it is OK to use T as the type parameter name. If there are multiple type parameters, however, we recommend being a little more descriptive. Generic types with multiple generic type parameters should name their parameters starting with "T" and suffixed by some short abbreviation describing the type's purpose. For example, in a dictionary type, we would have TKey and TVal as the generic types for the key and value respectively. Even for the single parameter case, a descriptive generic parameter name is sometimes better than just T . |
Convention | Explanation |
---|---|
Name methods using given, when, then format | This format makes it easier to understand the purpose of the test method without reading the code. For example, do not use method names such as: methodATest . Instead, name them something like: Given_StartingCondition_When_SomethingHappens_Then_TheExpectedResultIsX
|
Use the AAA pattern for tests (#30) | The idea is to use comments to split the test method into 3 sections: "Arrange", "Act", and "Assert". This convention makes it easier to get a high level view of what the test is doing and forces the code in the test to follow a logical sequence. Note: the arrange, act, assert sections correspond to the given, when, then conditions of our test, respectively. The arrange section produces the given condition. Assertions may be carried out in this section but any such assertions should start with the uppercase label "GIVEN: ". This ensures that it's obvious when a test fails due to poor arrangement code rather than the act code itself failing. The act section in turn should call upon the unit of functionality being tested. The assert section then contains assertions about the post-condition after the act e.g. checking that some object's internal state has been modified to satisfy some condition, or checking that an object returned by a method satisfies some condition. Assertion messages are optional, but often encouraged, if they make the failure clearer: think about someone running your test many years in the future. Will they understand a failure if it occurs? Sometimes, the assemble section of a test might be empty. Or the assert/act sections might be squashed into one. In cases such as these, the convention is to leave out the comments: a comment demarcating an empty section could just lead to confusion. |
...More conventions will follow. Please be patient with us as we convert our coding experience to text...