Code Reviews in Practice

Code Reviews

Code reviews are an invaluable practice that many software development teams often over look. This article explains how to put them in practice. The hope is, by establishing the key elements of code reviews, developers can more confidently work code reviews into their software development process and team environments.

Why do we do code reviews?

Code reviews have a number of benefits. A small list of the benefits include:

  • A different or new perspective can spot potential issues that the original developer may not have seen.
  • Reviewers bring experience from past projects and can identify common pitfalls that they have had to work through.
  • Learning can go two ways: Both reviewees and reviewers have potential for learning patterns from each other.
  • Socialization of how new software is written. This allows us to gain insight into what projects have solved what problems, and having the ability to reference that code for use in future projects.
  • Knowing that your code will be reviewed can lead to better programming practices.

What does a code review entail?

Ideally, code reviews are no more than a dialog between two or more developers about a finite amount of newly written code. (“Just talking ’bout some code bro.”) The amount of code should be small enough so that the code review can be done relatively quickly and encapsulates a related set of features.

The developer who wrote the code should provide reviewer(s) with relevant background information about the problem that needed to be solved and then proudly speak to and show the code that was written. The crucial keyword at this step is proudly. Before getting to the point of code review, developers should ensure that the code they are writing lives up to their team’s code quality standards and they are also satisfied with the pattern of implementation. If they are not able to come to satisfactory pattern for implementation, reaching out to team members for help is always a good choice. Better to ask for another eye on a problem early than to waste time and brain cells on something that ultimately will need to be refactored.

Once the code has been spoken to, it is the reviewers job to ask questions. Coming up with and asking questions is the key to to successful code reviews as it challenges both the original developer and the reviewer to look at the problem being solved and the code in a new light.

In addition to asking good questions about the code, there are a few other questions reviewers should be asking themselves:

  • Is the code easy understand? Should it be refactored or are comments needed?
  • Are there any areas that can be optimized?
  • Is there repetitive code that can be broken out into a re-usable method?
  • Are coding standards being followed?
  • What can I learn from this?

When should we do code reviews?

Just like voting in Illinois, code reviews should be done early and often. A good rule of thumb is any time you are creating new code that is adding a feature or solving a problem in existing software, you should ask for a code review. In addition, it can also be useful from time to time to ask for a code review on common programming tasks.

Who should do code reviews?

Everyone. Literally every developer regardless of levels of experience should ask for code reviews.

Code reviews are NOT a judgement on the code being reviewed

Finally and perhaps often overlooked, the intent of code reviews is NOT to cast judgement on the work product that your peer developer has worked so hard on. Plain and simple, don’t be a jerk and also don’t be afraid about your work being judged.

Offline Web Apps – Part 2: navigator.onLine

In Part 1 of this two part blog post, I covered a few of the basic fundamentals of offline web apps. Specifically, how to use a cache manifest to cache application resources offline in the browser. In part 2 we will dive into the how to ensure your application is intelligent enough to handle offline scenarios using navigator.onLine.

Meet navigator.onLine

Caching application resources is a nice feature provides end users with the ability to have client side interactions with your application while they are offline. But what happens when your application needs to communicate with a server? To do so, you need to write javascript code that will take into account how to handle user driven events based on online/offline status. With the HTML5, there is a helpful state property of the Navigator object onLine along with two events that get fired (ononline/onoffline) when the connection status changes.

A Real-world Example

Let’s say for example you have a client side application where users enter large amounts of text content that gets saved to a server somewhere. In order to prevent users from losing work by an unintended browser close, you want to occasionally “autosave” their data every 30 seconds. This code might look something like this:

window.addEventListener('load', function () {
    var saveInterval = setInterval(function () {
        // Function call (XHR etc) to save data goes here
        ...        
    }, 30000) // Call this function every 30 seconds
});

Unfortunately, as straightforward as this implementation is, it leaves some pretty big gaps:

  • The function call to save data will repeat every 30 seconds regardless of success/failure
  • Users have no idea whether or not their data is indeed being saved to the server

Let’s fix this by using navigator.onLine and the ononline/onoffline events:

window.addEventListener('load', function () {           

    function updateOnlineStatus() {
        var condition = navigator.onLine ? "online" : "offline",
            saveInterval = null;
        if (condition === "online") {
            // Make update to UI here to let user know they are online and autosave is available
            ...
            ...
            saveInterval = setInterval(function () {
                    // Function call (XHR etc) to save data goes here
                       ...
                       ...
                    }, 30000)
                }
                else {
                    // Make update to UI here to let user know they are offline and are not autosaving
                    ...
                    ...
                    // Stop the interval from repeating until connectivty is resolved
                    clearInterval(saveInterval); 
                }
            }

        // Setup event listeners for when online/offline status changes
        window.addEventListener('online', updateOnlineStatus);
        window.addEventListener('offline', updateOnlineStatus);

        // Make initialization call to updateOnlineStatus
        pdateOnlineStatus();
});

With the code above, we have pretty solid coverage for when (and when NOT) to save changes based on connection status. As you can imagine though, the more complex an application is, the more complex these scenarios might become.

Additional Considerations and Resources

Like any other that gets executed in the browser, there are some subtleties to watch out for when it comes to cross browser compatability. At this time of this writing Firefox ignores actual network connectivity and instead triggers on the browser’s “Work Offline” mode. In addition, some browser/operating system combinations may not be intelligent enough to determine the difference between a true network connection and a local network adapter (virtual) showing an online status. For more information, please take a look at the following links:

Offline Web Apps – Part 1: Cache Resources

"No internet connection"

Offline web apps seems like a contradictory notion. After all, the web is based on the interconnectivity of devices. However, as we push the limits of when and where users access web content, “offline” web applications are drawing an increased demand. By taking advantage of a few different browser features (like window.applicationCache or navigator.onLine) providing dual mode (online/offline) applications an easier than expected task.

Basics of an Offline Application

Dual mode web applications at the bare minimum must satisfy two fundamental tasks:

  1. Caching of application dependent resources
  2. Be intelligent enough to handle both online and offline scenarios

Depending on the complexity of your web application, these two fundamentals may have varying levels of difficulty but the core concepts are the same.

Cache Application Resources

For initial load of an application, end users must have an online connection to access resources from the server. It is at this time where you can configure your application to let the browser know what files to keep a copy of locally in case your connection is dropped. This is done by using a cache manifest. The cache manifest is a simple text file with an “appcache” extension referenced as an attribute to your top level html tag.

<html manifest="manifest.appcache">
    <head>
        ...
    </head>
    <body>   
        ...
    </body>
</html>

At the most basic level your cache manifest file should list the files required to run your application in an offline mode under a “CACHE MANIFEST” header. These files will be stored locally on the device and the next time an application is accessed, the browser will know to use the cached file if available. (Note: This is also a very helpful tool for improving online application performance.)

CACHE MANIFEST
index.html
scripts/app.js
scripts/data.js
css/app.css
images/logo.png
...

For more information on using a cache manifest, check out Appcache Facts.

Part 2: Handle Offline Scenarios

Unlike setting up an application cache, configuring your application to handle offline scenarios is much more application specific in it’s implementation. In Part 2 of this post, I’ll cover how to handle those scenarios.

Ready to see more? Check out Part 2 of this post learn about navigator.onLine.