The Software Scrutiny Journal

When implementing separation of concerns, there are many ways to skin a cat. Code can be split, decoupled, and moved around in many different ways. However, not all decoupling is equal. Some can lead to great design, while others may lead to a good deal of technical debt.

I found myself in a predicament when I was hunting for the source of a bug, and it was buried in an incorrectly decoupled system. To give a little background, this system was preparing attributes to be dynamically rendered as a form.

Some kinds of attributes displayed in the form are:

• percentages
• temperatures
• enums
• strings
• float.

As you can see, some of these items are data types with a specific unit attached to it (i.e. degrees), while others are just primitive types (i.e. float).

It’s also important to note that the construction of these attributes depend on many different sources of data. These would include:

1. basic device attribute information
2. If attribute is a number with a minimum/maximum value, get these minimums and maximums from a store that is user-specific.
3. Fetch the human readable label and description from the attribute display store. Unfortunately, without going into specifics, this cannot be included in #1.

The yucky technical debt.

Initially, I found the system “decoupled” by attribute type (percent, temperature, dropdown, enum, toggle, etc). So we had a set of functions that looked a little like this:

const inputForType = {
  percent: (attribute, userSpecific, display) => {
    const { type } = attribute
    const min = userSpecific.min
    const max = userSpecific.max

    let defaultValue
    if (userSpecific.defaultValue) {
      defaultValue = userSpecific.defaultValue
    } else if (attribute.defaultValue) {
      defaultValue = userSpecific.defaultValue
    } else {
      defaultValue = someDefaultValueByType(type)
    }

    let value = attribute.value || defaultValue

    // convert into a readable percentage
    value *= 100
    defaultValue *= 100

    const { description, label } = display[attribute.name]

    return {
      type,
      min,
      max,
      defaultValue,
      value,
      label,
      description,
    }
  },
  temperature: (attribute, userSpecific, display) => {
    // This is the exact same as percent, except we convert the value to
    // Fahrenheit if the user specifies.

    const { type } = attribute
    const min = userSpecific.min
    const max = userSpecific.max

    let defaultValue
    if (userSpecific.defaultValue) {
      defaultValue = userSpecific.defaultValue
    } else if (attribute.defaultValue) {
      defaultValue = userSpecific.defaultValue
    } else {
      defaultValue = someDefaultValueByType(type)
    }

    let value = attribute.value || defaultValue
    const { description, label } = display[attribute.name]

    if (userSpecific.temperature === 'F') {
      value = convertCToF(value)
      defaultValue = convertCToF(defaultValue)
    }

    return {
      type,
      min,
      max,
      defaultValue,
      value,
      label,
      description,
    }
  },
}

Why this is bad.

I am not going to implement the other types since I think you get the idea. The important parts to highlight is that the return value abides by the same schema, and so do the function parameters.

The other important bit to highlight is that we are repeating so much darn code for each data type. Setting defaultValues, min/max, and values is basically a copy-paste job. So even though this is “decoupled” by the type of attribute, this is not sufficiently applying DRY principals.

Finally, our dynamic form rendering engine (not included in the code above), will separately check for the input type, and render the input based on the given information. So if we provide a minimuim and maximum attribute to a string input component, it’s safe to say that the string input component will ignore any extra, unneeded attributes.

This means that we can unify all of the above functions into one, and then decouple our logic based on fetching the value, defaultValue, conversions, etc.

How to improve.

So what would this look like? I’m glad you asked.

const getConverter = (userSpecific, displayType) => (value) => {
  const { temperature } = userSpecific

  if (displayType === 'temp' && temperature === 'F') {
    return convertCToF(value)
  } else if (displayType === 'percentage') {
    return value * 100
  } else {
    return value
  }
}

const getDefaultValue = (attribute, userSpecific) => {
  let defaultValue
  if (userSpecific.defaultValue) {
    defaultValue = userSpecific.defaultValue
  } else if (attribute.defaultValue) {
    defaultValue = userSpecific.defaultValue
  } else {
    defaultValue = someDefaultValueByType(type)
  }

  return defaultValue
}

const inputForType = (attribute, userSpecific, display) => {
  const { type } = attribute
  const convert = getConverter(userSpecific, display.displayType)

  const min = userSpecific.min
  const max = userSpecific.max

  const defaultValue = convert(getDefaultValue(attribute, userSpecific))
  const value = attribute.value ? convert(attribute.value) : defaultValue
  const { description, label } = display[attribute.name]

  return {
    type,
    min,
    max,
    defaultValue,
    value,
    label,
    description,
  }
}

These may look similar according to the amount of code, however I can guarantee to you that when there are 10-15 different combination of form types and input types, things get messy very quickly.

So let’s say we want to add a new type called delta degrees. This is very similar to converting temperatures, however for the delta degrees, we are not offsetting by 32. This is so that the user can specify the change in degrees.

The only code that needs to be augmented is the getConverter function, and everything else just works!

const getConverter = (userSpecific, displayType) => (value) => {
  const { temperature } = userSpecific

  if (displayType === 'delta_temp' && temperature === 'F') {
    return convertDeltaCToF(value)
  } else if (displayType === 'temp' && temperature === 'F') {
    return convertCToF(value)
  } else if (displayType === 'percentage') {
    return value * 100
  } else {
    return value
  }
}

Adding this new type only required the addition of 2 lines of code. If we compare this to the legacy code, it would have required the addition of about 30 lines of code. It’s easy to see how this can get out of hand very quickly.

Next time you find yourself in one of these patterns, take a look at the structure of your data. If you are able to recombine everything and separate the concerns another way, you might just save yourself from a mountain of technical debt.