A Cleaner Code Case Study

A Cleaner Code Case Study

I recently had a situation at work wherein a coworker tried to modify a JavaScript function that I had written and ended up introducing some bugs. In reviewing their code, it seemed that their main issue was in not fully understanding what the function was doing. I do, however, believe that it was my fault because the function was — frankly — poorly written to begin with.

I recently had a situation at work wherein a coworker tried to modify a JavaScript function that I had written and ended up introducing some bugs. In reviewing their code, it seemed that their main issue was in not fully understanding what the function was doing. I do, however, believe that it was my fault because the function was — frankly — poorly written to begin with.

Sometimes we have deadlines, and, in order to meet them, we may leave things a mess. I had plans to revisit the code but other tasks took priority. Now that the function was back knocking on my door I saw an opportunity to fix it. Often when we share our code with the world, we share our most meticulously maintained material. But that is not the reality of business all the time. At the end of the day, the product and the customers that use it are the priority and when it comes to deadlines vs perfectly clean code, the deadline wins. However, when we get the chance to go back and clean up after ourselves we should take those opportunities because it’s important that we balance production with our capacity to continue producing.

I’m going to attempt to remedy the diseased function in steps in order to give you an example of how I go through the process of improving code.

The original code

Let’s now look at the original function that gave my fellow developer problems:

function valid(field, visibleField) {
   var state = {
      saved: true,
      requirements: {
         Description: {
            required: true,
            maxlength: 150
         },
         DueDate: {
            date: true
         },
         PriorityID: {},
         TypeID: {}
      }
   };

   if (!state.requirements[field.name]) {
      return true;
   }

   var errorField = visibleField ? visibleField : field;

   // required
   if (state.requirements[field.name].required) {
      if (field.tagName.toLowerCase() == 'input' && field.value.length == 0) {
         errorField.classList.add('inputBorderError');
         return false;
      } else if (field.value === undefined || field.value === '') {
         errorField.classList.add('inputBorderError');
         return false;
      }
   }

   // max length
   if (state.requirements[field.name].maxlength) {
      if (field.value.length > state.requirements[field.name].maxlength) {
         errorField.classList.add('inputBorderError');
         return false;
      }
   }

   // date
   if (state.requirements[field.name].date) {
      if (!moment(field.value, ['MM/DD/YYYY', 'YYYY-M-D'], true).isValid()) {
         errorField.classList.add('inputBorderError');
         return false;
      }
   }

   errorField.classList.remove('inputBorderError');
   return true;
}

Let me also provide some simplified HTML so you can see a sample of the function’s usage:

<form id="myForm">
    <div>
        <input 
            name="Description" 
            type="text" 
            oninput="
                if (valid(this)) { 
                    edit(this); 
                }
            "
        >
    </div>

    <div>
        <input 
            name="DueDate"
            type="text"
            oninput="
                if (valid(this, document.getElementById('myForm'))) { 
                    edit(this); 
                }
            "
        >

    </div>

    <button type="submit">Submit</button>
</form>

The function is decently complex, so let’s go over it to make sure we understand what’s happening. We have a valid() function that takes in the parameters field and visibleField. This is used within the context of an HTML form, so the two parameters are HTML elements. We see a variable immediately declared called state. It has a saved property and a requirements property.

// ...
requirements: {
   Description: {
      required: true,
      maxlength: 150
   },
   // ...
}

One of the immediate issues you may notice is that the saved property in state isn't even used. Instead of confusing you by explaining its original purpose, let's just accept that there was a plan for it upon initial development that was since abandoned, making the saved property an artifact of an old design (it never was cleaned out).

// max length
if (state.requirements[field.name].maxlength) {
   if (field.value.length > state.requirements[field.name].maxlength) {
      errorField.classList.add('inputBorderError');
      return false;
   }
}

The keys in the requirements property in the state object are mapped to field names in the form ( Description and DueDate are in our HTML form). The requirements properties' values, which are objects, map to different validations we want to perform on the field. For example, if we have...

…our max length if-block catches it and returns false if it fails.

We can also see that the function handles displaying the error by adding a class to an element ( errorField.classList.add('inputBorderError')). If a visibleField element is provided, that is what the error is displayed on, otherwise it uses the primary field element.

If the field passes through all of the validation rules that apply to it without returning false, the function eventually returns true, so the function always returns a boolean.

clean-code refactoring javascript code code-review

Bootstrap 5 Complete Course with Examples

Bootstrap 5 Tutorial - Bootstrap 5 Crash Course for Beginners

Nest.JS Tutorial for Beginners

Hello Vue 3: A First Look at Vue 3 and the Composition API

Building a simple Applications with Vue 3

Deno Crash Course: Explore Deno and Create a full REST API with Deno

How to Build a Real-time Chat App with Deno and WebSockets

Convert HTML to Markdown Online

HTML entity encoder decoder Online

Softagram - Making Code Reviews Humane

The story of Softagram is a long one and has many twists. Everything started in a small company long time ago, from the area of static analysis tools development. After many phases, Softagram is focusing on helping developers to get visual feedback on the code change: how is the software design evolving in the pull request under review.

Effective Code Reviews: A Primer

Peer code reviews have increasingly been adopted by engineering teams around the world. Here are 6 tips to make the process better for teams.

How to Find the Stinky Parts of Your Code (Part II)

There are more code smells. Let’s keep changing the aromas. We see several symptoms and situations that make us doubt the quality of our development. Let's look at some possible solutions.

Static Code Analysis: What It Is? How to Use It?

Static code analysis is a method of debugging by examining source code before a program is run. It's done by analyzing a set of code against a set (or multiple sets) of coding rules. Static code analysis and static analysis are often used interchangeably, along with source code analysis.

4 Ways You Can Get Rid of Dirty Side Effects for Cleaner Code in JavaScript

4 Ways You Can Get Rid of Dirty Side Effects for Cleaner Code in JavaScript. Bugs are born in many ways. Creating side effects is one of them. Some people say side effects are evil, some say they’re not.