This blog describe some tips and experience about the code review.
Naming convention
For example, in some c++ code, it uses the camel cases, for other code, it may use the underscore convention. Make sure that the parameter should be consistent with the existing coding style. This is the easist thing which only need some patience to do so.
For example, this is the coding style convention of VTK and VTKm, it is important to keep them in mind before and check each item before submitting the code. Especially for when to use capital letter and camel case. For example, local variables should start in lower case and then to use camel case.
using existing mechanism
When trying to add some new functions, the first step is trying to figure out if the current code base or framework provide these type of functions or mechanism. Do not introduce new features only if it is necessary. A simple case is in vtkm filter, the base class of filter provides the ActiveField parameter. So try to use this instead of setting a new parameter in the customized filter class.
using worklet instead of for loop
This is in the context of the vtkm’s code. When there is a for loop, we always try to consider if it is ok to use worklet to do so to run it in parallel on specific devices instead of using the for loop, there are some testing helper function such as test_equal_arrayHandle which can compare two arrays to check if they equal with each other.
Avoid using unnecessary variables
I tend set a bool variable and update the bool variable in the function and return that bool varibale at last. Actually, we could just return the true or false direactly instead of returning that variable at last, we just return that bool variable when it is possible.
Long function in hierarchical layout
I was working on a function that tries to merge multiple vtkm data set in one data set. Originally, I put everything in a do merge function, even if I add enough comments in each subsection of that function, but it is still vary long, the code is hard to maintatin and some variable defined at the beginning might be used at the last part of the function. Based on the advice from the senior engineer, I break it into several part, namely checking the edge case of the input data, merge connectivity, merge field and merge coordinates system etc. The do merge function contains the main skeleton of the code, each subsection is a particular function that calls a subroutine. After doing this, the code is much more clear and each function basically do one thing, the logic is more clear.
Sometimes, writting code is just like writting paper, you want to make sure each paragraph only discuss one thing. Either the summary of whole section or explain the specific one point. With this idea in mind, the code can be more easily to read and maintain in future.
