Perfectly Broken Code

Written by Gerardo Rodriguez on

A cute, smiling bug between two code brackets

Naturally, as a Senior Front-End Developer with over a decade of experience, the code that I write is flawless and bug-free all day, every day. LOL —Β Just kidding.Β πŸ˜… πŸ¦„

Even though I have many years of experience, I write code that breaks every single day. There’s no magical transformation from when I first started writing code to today. I’ve realized and learned that writing broken code is part of the process; it’s all about iteration. With the experience I’ve gained, I do see patterns quicker. But I still make mistakes, especially in the first iteration.

Join me in exploring a recent experience where I started with flawed logic (without realizing it) and the steps I took to fix my bug. Let’s experience some broken code together. πŸŽ‰

The goal

My goal is to write a JavaScript utility function, getRenderOptionsFromEl(), that reads specific custom data attributes to create and return a JavaScript object literal with boolean values.

From this source HTML:

<div class="widget js-widget" 
  data-widget-id="123" 
  data-render-call-to-action-button="true"
  data-render-flags="false" 
  data-render-logos="false" 
  data-render-title="false" 
  data-other="Some other data" 
>
  <!-- Widget HTML -->
</div>

I want to generate a JavaScript object literal with only the render* properties and values as booleans:

{
  renderCallToActionButton: true,
  renderFlags: false,
  renderLogos: false,
  renderTitle: false
}

Requirements:

  1. All custom data attributes will start with data-render-* as a naming convention.
  2. The following data-render-* attributes can optionally exist for a given widget:
    • data-render-call-to-action-button
    • data-render-flags
    • data-render-logos
    • data-render-title
  3. For each data-render-* attribute, its value will either be a string of "true" or "false".
  4. There may be one or more widgets on the page.
  5. The getRenderOptionsFromEl() function should accept an HTMLElement argument and return an object literal with boolean values assigned to the render* properties.

Solution one: I thought it was working

Assuming the source HTML from above, I can access the data-render-logos="false" data attribute directly using the dataset read-only property:

const el = document.querySelector('.js-widget');

if (el) {
  // The dataset dot-notation is so smooth...😎
  const shouldRenderLogos = el.dataset.renderLogos; 

  console.log(shouldRenderLogos); // "false"
}

Sweet! Following this approach, the first implementation of the utility function was as follows:

// scripts/utils/get-render-opts-from-el.js

/**
 * Generates an object literal with render option properties
 * 
 * @param {HTMLElement} el The element to read data attributes from
 * @returns {Object} An object of boolean render options
 */
export const getRenderOptionsFromEl = el => ({
  // For each property, set its value to a boolean by 
  // checking if the dataset property is 'true'.
  renderCallToActionButton: el.dataset.renderCallToActionButton === 'true',
  renderFlags: el.dataset.renderFlags === 'true',
  renderLogos: el.dataset.renderLogos === 'true',
  renderTitle: el.dataset.renderTitle === 'true'
});

Now that we have the utility function, let’s test it out against an HTML element on the page:

<!-- index.html -->

<div
  class="widget js-widget"
  data-widget-id="123"
  data-render-call-to-action-button="true"
  data-render-flags="false"
  data-render-logos="false"
  data-render-title="false"
  data-other="Some other data" 
>
  <!-- Widget HTML -->
</div>

<!-- 
  Modern browsers support JavaScript modules natively
  @see https://developer.mozilla.org/en-US/docs/Web/JavaScript/Guide/Modules#Applying_the_module_to_your_HTML
  @see https://caniuse.com/#feat=es6-module
-->
<script type="module" src="scripts/index.js"></script>

And here’s the scripts/index.js JavaScript file that imports and uses the utility function:

// scripts/index.js

import { getRenderOptionsFromEl } from './utils/get-render-opts-from-el';

document.querySelectorAll('.js-widget').forEach(el => {
  // Get the render options for each widget
  const renderOpts = getRenderOptionsFromEl(el);

  // Let's see what the renderOpts object looks like!
  console.log(`Widget ${el.dataset.widgetId}`, renderOpts);
});

Which logs out to the browser console:

// browser console

Widget 123
{
  renderCallToActionButton: true, 
  renderFlags: false, 
  renderLogos: false, 
  renderTitle: false
}

Yay, it worked! Goal accomplished. πŸŽ‰ You can see it running in the browser as well:

A browser screenshot with the console log showing expected output for solution one

Time to add multiple widgets

Seeing the logic works as designed, I continue by adding a second widget onto the page representing a second use case, a widget with a limited set of custom data-render-* attributes. The updated source HTML looks as follows:

<!-- index.html -->

<!-- First widget with all four render data attributes -->
<div
  class="widget js-widget"
  data-widget-id="123"
  data-render-call-to-action-button="true"
  data-render-flags="false"
  data-render-logos="false"
  data-render-title="false"
  data-other="Some other data" 
>
  <!-- Widget 123 HTML -->
</div>

<!-- Second widget with only two render data attributes -->
<div
  class="widget js-widget"
  data-widget-id="456"
  data-render-flags="false"
  data-render-logos="false"
  data-are-you-having-fun="Absolutely!"
>
  <!-- Widget 456 HTML -->
</div>

Taking a peek at the browser console, I see the following:

// browser console

Widget 123 
{
  renderCallToActionButton: true, 
  renderFlags: false, 
  renderLogos: false, 
  renderTitle: false
}

Widget 456 
{
  renderCallToActionButton: false, 
  renderFlags: false, 
  renderLogos: false, 
  renderTitle: false
}

Oh no, there is a bug! 😱

Notice the second widget, Widget 456, only has two custom data-render-* attributes in the source HTML:

  • data-render-flags="false"
  • data-render-logos="false"

This means the expected output in the browser console for the second widget should be:

Widget 456 
{
  renderFlags: false, 
  renderLogos: false, 
}

Broken code represents opportunities

The first iteration of the utility function logic is flawed. Where do I go from here? Well, I fix it, of course. Before doing so, I see a great opportunity at this point in the process of adding some test assertions. Before this, I didn’t write a test, and that was okay. The logic seemed pretty straightforward, and 100% code coverage isn’t the end-all-be-all. Tests should add more confidence that your code’s logic will work as expected, and I believe that testing for use cases provides greater confidence. Since the logic failed when I put it through the second use case, I’ve lost confidence. Time to boost my confidence!

Using Jest, I’ll add a baseline test for the first use case of having only one widget with all four custom data-render-* attributes:

// scripts/utils/get-render-opts-from-el.test.js

// Import the utility function we intend to test
import { getRenderOptionsFromEl } from './get-render-opts-from-el';

test('getRenderOptionsFromEl() should return four attributes', () => {
  // Create a mock element to test against
  const el = document.createElement('div');
  // Add mock custom data attributes
  el.setAttribute('data-widget-id', '321');
  el.setAttribute('data-render-call-to-action-button', 'true');
  el.setAttribute('data-render-flags', 'false');
  el.setAttribute('data-render-logos', 'false');
  el.setAttribute('data-render-title', 'true');
  // I add "other" custom data attributes to confirm 
  // they are ignored by the utility function.
  el.setAttribute('data-other', 'Some other value');

  // Use the utility function!
  const result = getRenderOptionsFromEl(el);
  // Set the expectations on what the result _should_ be
  const expected = {
    renderCallToActionButton: true,
    renderFlags: false,
    renderLogos: false,
    renderTitle: true
  };

  // Finally, write the assertion
  expect(result).toEqual(expected);
});

The test passes, as expected, which we verified in the browser for this first use case:

'getRenderOptionsFromEl() should return four attributes' test passes

Next, I add a new test for the second use case where the bug was discovered:

// scripts/utils/get-render-opts-from-el.test.js

test('getRenderOptionsFromEl() should only return two attributes', () => {
  // Create mock element that only has
  // two data-render-* attributes.
  const el = document.createElement('div');
  el.setAttribute('data-widget-id', '321');
  el.setAttribute('data-render-call-to-action-button', 'false');
  el.setAttribute('data-render-flags', 'false');
  el.setAttribute('data-other', 'Some other value');

  const result = getRenderOptionsFromEl(el);
  const expected = {
    renderCallToActionButton: false,
    renderFlags: false
  };

  expect(result).toEqual(expected);
});

And our new test fails, confirming the bug:

'getRenderOptionsFromEl() should only return two attributes' test fails

Before we continue, I want to be clear; a failing test is a good thing! I now have what I need to gain my confidence back in tests to help guide me through the refactor. πŸ˜ƒ

Solution two: The refactor

With the failing test guiding me, I went ahead and refactored the utility function. I took the opportunity to strengthen the JSDoc @returns type by adding a custom RenderOptions @typedef (Type Definition) to help document the new logic:

// scripts/utils/get-render-opts-from-el.js

/**
 * A type definition to provide clear expectations 
 * of the object and its properties
 * 
 * @typedef {Object} RenderOptions
 * 
 * All of the render* properties are optional
 *
 * @property {boolean} [renderCallToActionButton]
 * @property {boolean} [renderFlags]
 * @property {boolean} [renderLogos]
 * @property {boolean} [renderTitle]
 */

/**
 * Generates an object literal with render option properties
 *
 * @param {HTMLElement} el
 * @returns {RenderOptions} 
 */
export const getRenderOptionsFromEl = el => {
  // The object that'll eventually be returned
  const renderOptions = {};

  // Conditionally add the render* properties only if 
  // the data-render-* attribute exists on the DOM element.

  if (el.dataset.renderCallToActionButton) {
    const value = el.dataset.renderCallToActionButton === 'true';
    renderOptions.renderCallToActionButton = value;
  }

  if (el.dataset.renderFlags) {
    const value = el.dataset.renderFlags === 'true';
    renderOptions.renderFlags = value;
  }

  if (el.dataset.renderLogos) {
    const value = el.dataset.renderLogos === 'true';
    renderOptions.renderLogos = value;
  }

  if (el.dataset.renderTitle) {
    const value = el.dataset.renderTitle === 'true';
    renderOptions.renderTitle = value;
  }

  return renderOptions;
};

Cool, let’s take a peek at our tests now:

'getRenderOptionsFromEl() should only return two attributes' test passes

Alright! Both tests are now passing. πŸŽ‰ We can confirm this via the browser as well:

I fixed the broken code, but…

The second iteration of my solution is better than the first one. But I couldn’t help noticing it isn’t the most flexible. Consider the following: What if the requirements change, as they often do, where the design needs to support five, six, or an unknown amount of attributes with unknown names? It’s to nobody’s benefit for a developer to keep going back and adding one-off conditional checks for each new data-render-* attribute that might be added in the future.

Ideally, the solution should minimize the chance that further maintenance or one-off conditionals need to be added down the line. I decided to look at one more solution iteration.

Embrace the Test-Driven Development spirit

Before working on a third iteration, I embrace the Test-Driven Development (TDD) spirit and first add a failing test to help guide me:

test('getRenderOptionsFromEl() handles unknown attributes', () => {
  // Create mock element
  const el = document.createElement('div');
  // Add data attributes
  el.setAttribute('data-widget-id', '321');
  el.setAttribute('data-render-flags', 'false');
  el.setAttribute('data-other', 'Some other value');
  // Add a couple of new, unknown data-render attributes!
  el.setAttribute('data-render-the-future', 'true');
  el.setAttribute('data-render-the-unknown', 'true');


  const result = getRenderOptionsFromEl(el);
  const expected = {
    renderFlags: false,
    renderTheFuture: true,
    renderTheUnknown: true
  };

  expect(result).toEqual(expected);
});

And, confirming my hunch, the test fails:

'getRenderOptionsFromEl() handles unknown attributes' test fails

Perfect! With the confidence-boosting failing test in my back pocket, I excitedly move forward toward a third iteration. πŸ™‚

Solution three: For the win!

While this might’ve been the most challenging solution to come up with, I enjoyed it the most when it came to problem-solving for it. After layered progress in the form of failures, the following solution eventually came to light:

// scripts/utils/get-render-opts-from-el.js

/**
 * @typedef {Object} RenderOptions
 * @property {boolean} [renderCallToActionButton]
 * @property {boolean} [renderFlags]
 * @property {boolean} [renderLogos]
 * @property {boolean} [renderTitle]
 */

/**
 * Generates an object literal with render option properties
 *
 * @param {HTMLElement} el
 * @returns {RenderOptions}
 */
export const getRenderOptionsFromEl = el => {
  const renderOpts = {};
  for (const attrKey in el.dataset) {
    // We only care for attribute keys that
    // begin with 'render', per the requirements.
    if (attrKey.startsWith('render')) {
      const attrValue = el.dataset[attrKey];
      // Add the attribute key and value, as a boolean,
      // to the output object.
      renderOpts[attrKey] = attrValue === 'true';
    }
  }
  return renderOpts;
};

A final test run

Okay! Now let’s take a peek at the tests:

All tests are passing!

Yay, they all pass! πŸŽ‰ Allow me a moment to high-five myself. πŸ€“

We can confirm in the browser as well:

A browser screenshot with the console log showing expected output for solution three

Web development is a craft, enjoy the journey

Some key moments that stood out for me as I iterated toward a solution:

  • Bugs provide an excellent opportunity to add tests. The tests will guide you during your logic refactor to fix the bug and future-proof your code from regressions.
  • I found an opportunity to use the spirit of Test-Driven Development. I think it’s neat to sprinkle in moments to use this strategy without needing to subscribe to an all-or-nothing approach.

In sharing my experience, I hope to show how our first attempts at a solution aren’t always correct. And that that’s okay! Just like writing an essay, it’s about getting the first draft out then iterating on it. As a developer, it doesn’t matter if you’re getting started in your career or have plenty of years of experience, don’t get discouraged by writing broken code. Instead, embrace it and get excited about the opportunity to fix the logic flaw by iterating toward a better solution. πŸ™‚

Gerardo Rodriguez

Gerardo Rodriguez is a developer driven by curiosity and the love of learning. His experience with art, design, and development make him a well-rounded maker of things on the web. You’ll find him sharing some thoughts and resources on Twitter via @_gerardo.

Never miss an article!

Get Weekly Digests


Leave a Comment

Please be kind, courteous and constructive. You may use simple HTML or Markdown in your comments. All fields are required.


Let’s discuss your project! Email Us