Thursday, March 13, 2014

Avoid flag parameters in public methods

I was recently reading some code that I wrote a while back, and it looked something like this:
And then later on in the code I saw
At the time I wrote this, it was very clear what this meant, and I was saving myself time by having a single method where a flag would slightly alter the behavior. Reading this code several years later, it wasn't clear at all what it was doing. So how did I write it so it was easier to read? Instead of having the flag in the public method, just have two separate methods, and bury the flag in a private method. After refactoring, my code now looks like:
Internally, there is still a run method that takes a flag for running asynchronously, but the public API is now a lot easier to understand.

Tuesday, March 11, 2014

Effective Code Reviews

There are numerous ways to conduct code reviews, ranging from the very formal to a constant review with pair programming. In this post, I'll discuss techniques that I have used for code reviews and what I expect to get out of a code review. In an environment where there is not pair programming, to me, code reviews are a must. I've seen huge improvements in codebases from doing reviews, resulting in both less code being written as well as cleaner code.

There are several reasons why code reviews are important...

They do find some bugs - not all of them, especially if the reviewer is not particularly familiar with that area of the code - but they help to weed out some of the obvious ones. Even the best developers make mistakes and having a second set of eyes can help. You can use automated tools where possible to filter out some things, but another look at the code is invaluable.

They help familiarize reviewers with areas of the code they don't know as well. As a system grows larger, not everyone will know every part of the code. By doing reviews, you can widen your understanding of the code, and you may find code that is useful for something else you're working on later on in time. For example, I've worked on larger codebases where you see similar utility methods in several different places because people didn't know they exist.

They socialize conventions. As you review other people's code and vice versa, you will start to pickup conventions that other developers use. Over time, you can end up with a more consistent looking codebase, making it easier to read, regardless of the author.

They are learning experiences. I've often been reviewing someone else's code and learned something I didn't know, such as a technique for doing something in whatever language you're using.

Peer pressure helps produce cleaner code. If I write something poorly, such as not well tested or well documented, I know the review will be rejected. While I like to think I always write everything the best I can, this is a nice reminder that other people will be reviewing the code.

How to conduct a review...

We've all probably done the big formal, ceremonious review at one time or another. Several days before the review, someone sends out the code to be reviewed or even prints it out and drops it off at your desk. You're expected to come to the review having reviewed the code already. In these situations, it has been my experience that only a small percentage of the people do a thorough review, and many reviewers simply show up without having read the code ahead of time or noting very trivial things like "this comment is misspelled." I think these types of reviews are generally a waste of too many people's time.

Currently I use Atlassian's Crucible to do reviews. When I am ready for my code to be reviewed, I select one or two people and add them to the review. They receive automatic notifications that they have a review waiting and they have some amount of time to complete it. The reviewers add comments to code and can optionally raise defects in JIRA. With this setup, people can review at their own pace. Similar to one benefit of code reviews, the peer pressure of having your comments publicly visible tends to make for more through and thoughtful reviews. I try to avoid long threads of comments on the review because tone can often be misconstrued. Any more than 1 or 2 comments, and I'll just have an in person discussion.

These are just my experiences with code reviews, and yours may vary. Feel free to comment to add to or disagree with anything I've said here.

Wednesday, December 25, 2013

Gradle daemon

The gradle daemon is a feature in gradle that is designed to speed up build times. A daemon process will run the builds, which reduces the startup time of the build. In an environment where you are running builds many times a day, this can be a significant time saver.

To enable the daemon, add the following property to your GRADLE_OPTS or

Gradle will automatically use the daemon process for the build. The daemon is killed off if it is not used for a while, but when that happens, the next build will spin up a new daemon. There are also additional options for not using the daemon, stopping it and running in the foreground that can be found in gradle's documentation

Wednesday, November 13, 2013

People are not resources

It's very common for project managers to refer to employees as resources. The dictionary definition of resource is something that can be used for support or help. People are people, not things. A chair is a resource. A computer is a resource. A person is not a resource - a person is a person.

By using the term resource, the individuality of people and the value they provide are trivialized. You can swap one similar chair for another, but can you really swap one "similar person" for another?

This may seem like a small thing, but it's really a small thing to start calling people people, and it can make a big difference.

Sunday, October 20, 2013

Keep methods short

I was reading some code recently, and saw the notation of marking the beginning and end of loops (not the first time I've seen this).

for ( int i = 0; i < someEndVal; i++ ) { // begin myProcessingLoop
  // long set of steps here
} // endMyProcessingLoop

Why is that information there? Usually because the processing loop is so long that it doesn't fit on a single screen, or because there are so many nested loops that it is hard to decipher. Instead of commenting on the loops like that, rewrite the loops so they are short. A common technique is to extract the loop body into its own method.

for ( int i = 0; i < someEndVal; i++ ) {

It removes the unnecessary comments and makes the code generally easier to read.

Wednesday, June 26, 2013

NoSQL is not schemaless

NoSQL datastores give us a lot of flexibility when it comes to putting data in. They don't need a fixed schema to accept data, and they're pretty good about handling data with mixed attributes. Consider I insert the following data into a mongodb store (I'll use Mongo to illustrate my point, but this could apply to most NoSQL stores):
[ { "first_name" : "Jeff", "last_name" : "Storey" } , { "color" : "red", "a_number" : 50 } ]
Sure, I can do this, but imagine trying to query this data. Mongo will let me write a query that selects all documents whose "first_name" is "Jeff" or whose "color" is "red," but this doesn't make much sense at an application level. In order for the data to be easily processed, it needs to conform to some schema.

Where having this flexibility is really useful is with a schema that changes over time - and what schema doesn't? Rather than having to add new columns, new constraints, etc to the database, mongo will happily allow you to add new columns. Now consider an original schema that looks like:
first_name, last_name, age
Then I realize I want to start collecting income data, and I change it to
first_name, last_name, age, income
For records that have income, just insert it. No database changes necessary!

Technically, yes, NoSQL stores can be schemaless, but that doesn't mean you shouldn't have a schema.

Saturday, April 6, 2013

Javascript Logging

When developing Javascript applications, we often need to do some logging and we usually resort to using console.log, which typically isn't a good thing to have in production code. I've been doing some research into Javascript logging frameworks, and there are quite a few of them.

log4javascript looks to be one of the more promising ones. It is still being actively updated, and the API is pretty straightforward, especially if you've used other logging frameworks like log4j. They even provide a default logger that requires no configuration to get running:

var logger = log4javascript.getDefaultLogger();
logger.debug("this is a debug message");

There are various types of appenders, including one that sends the log message through an ajax call to the server so the log message can be saved on the server.

If you're doing any sort of logging in your Javascript app, this is definitely worth a look.