Skip to main content

Meaningful Names - Code Review Checklist with Example

"Names are everywhere in software". "The hardest thing about choosing good names is that it requires good descriptive skills"[1]. In my previous post, I emphasized that we should take care our code, so now let's start with naming.

Issue
Original code
Revised code
Reveals nothing
int d; //elapsed time in days
int elapsedTimeInDays;
Difficult to understand
public List<Cell> getThem()

public List<Cell> getFlaggedCells()

Specific to programmers
accountList
accounts
Number-series
public void copyChars(char a1[], char a2[])
public void copyChars(char source[], char destination[])
Noise words (indistinguishable)
customerInfo vs customer;
accountData vs account
customer, account //distinguish names in such a way that the reader knows what the differences offer
Silly made-up words
class DtaRcrd02{
 private Date genymdhms;
}
class Customer{
 private Date generationTimestamp;
}
Poor name
for(int j=0 ;  j<34 ;  j++){
   s += (t[j]*4)/5;
}
int realDaysPerIdealDay = 4;
const int WORK_DAYS_PER_WEEK = 5;
const int NUMBER_OF_TASK = 34;
for(…)
Member prefixes
private String m_dsc; 
private String description;
Interfaces prefixes
IShapeFactory
ShapeFactory
Constructors with args
Complex point = new Complex(23.0)
Complex point = Complex.FromRealNumber(23.0)
//use static factory that describe the args
Form of colloquialisms or slang
eatMyShort()
abort()
Different methods name of different classes with same code base
fetch(), retrieve(), get()
get() //pick one word for one abstract concept and stick with it
Different class name in the same code base
Controller, manager, driver
Controller
The variable names are not clear when they are used alone in a method
street, houseNumber, city, state
//Solution 1: prefix addressStreet, addressHouseNumber, addressCity, addressState
//Solution 2: create a class Address

References:

Comments

  1. This is one of the best blog post on code review checklist. It is really very important to have best code review checklist to developers. Thanks for sharing.

    ReplyDelete

Post a Comment

Popular posts from this blog

AngularJS - Build a custom validation directive for using multiple emails in textarea

AngularJS already supports the built-in validation with text input with type email. Something simple likes the following:
<input name="input" ng-model="email.text" required="" type="email" /> <span class="error" ng-show="myForm.input.$error.email"> Not valid email!</span>
However, I used a text area and I wanted to enter some email addresses that's saparated by a comma (,). I had a short research and it looked like AngualarJS has not supported this functionality so far. Therefore, I needed to build a custom directive that I could add my own validation functions. My validation was done only on client side, so I used the $validators object.

Note that, there is the $asyncValidators object which handles asynchronous validation, such as making an $http request to the backend.

This is just my implementation on my project. In order to understand that, I supposed you already had experiences with Angular…

The HelloWorld example of JSF 2.2 with Myfaces

I just did by myself create a very simple app "HelloWorld" of JSF 2.2 with a concrete implementation Myfaces that we can use it later on for our further JSF trying out. I attached the source code link at the end part. Just follow these steps below:

1. Create a Maven project in Eclipse (Kepler) with a simple Java web application archetype "maven-archetype-webapp". Maven should be the best choice for managing the dependencies, so far. JSF is a web framework that is the reason why I chose the mentioned archetype for my example.

2. Import dependencies for JSF implementation - Myfaces (v2.2.10) into file pom.xml. The following code that is easy to find from http://mvnrepository.com/ with key words "myfaces".

<dependency> <groupId>org.apache.myfaces.core</groupId> <artifactId>myfaces-api</artifactId> <version>2.2.10</version> </dependency> <dependency> <groupId>org.apache.myfaces.core</groupId&g…

Only allow input number value with autoNumeric.js

autoNumeric is a jQuery plugin that automatically formats currency and numbers as you type on form inputs. I used autoNumeric 1.9.21 for demo code.

1. Dowload autoNumeric.js file from https://github.com/BobKnothe/autoNumeric
2. Import to project
<script src="http://ajax.googleapis.com/ajax/libs/jquery/1.11.0/jquery.min.js"></script> <script type="text/javascript" src="js/autoNumeric.js"></script> 3. Define a function to use it
<script type="text/javascript"> /* only number is accepted */ function txtNumberOnly_Mask() { var inputOrgNumber = $("#numberTxt"); inputOrgNumber.each(function() { $(this).autoNumeric({ aSep : '', aDec: '.', vMin : '0.00' }); }); } </script>
4. Call the function by event
<form> <input type="text" value="" id="numberTxt"/>(only number) </form> <script type="te…

Styling Sort Icons Using Font Awesome for Primefaces' Data Table

So far, Primefaces has used image sprites for displaying the sort icons. This leads to a problem if we want to make a different style for these icons; for example, I would make the icon "arrow up" more blurry at the first time the table loading because I want to highlight the icon "arrow down". I found a way that I can replace these icons with Font Awesome icons.


We will use "CSS Pseudo-classes" to achieve it. The hardest thing here is that we should handle displaying icons in different cases. There is a case both "arrow up" and "arrow down" showing and other case is only one of these icons is shown.

.ui-sortable-column-icon.ui-icon.ui-icon-carat-2-n-s { background-image: none; margin-left: 5px; font-size: 1.1666em; position: relative; } .ui-sortable-column-icon.ui-icon.ui-icon-carat-2-n-s:not(.ui-icon-triangle-1-s)::before { content: "\f106"; font-family: "FontAwesome"; position: absolut…

Attribute 'for' of label component with id xxxx is not defined

I got the warning in the log file when I have used the tag <h:outputLabel> without attribute "for" in xhtml file. It was really polluting my server log files.

The logged information actually makes sense anyway! We could find an answer as the following:

"Having h:outputLabel without a "for" attribute is meaningless. If you are not attaching the label, you should be using h:outputText instead of h:outputLabel."

However, these solutions are not possible just for my situation. Instead of using h:outputText for only displaying text, my team has used h:outputLabel too many places. We were nearly in our release time (next day) so it is quite risky and takes much efforts if we try to correct it. Because the style (with CSS) is already done with h:ouputLabel. The alternative by adding attribute "for" the existing h:outputLabel is not reasonable either. I really need to find another solution.
Fortunately, I came across a way if I change to use p:out…