Some errors to avoid in JavaScript

Published on December 12, 2019

Tagged: #engineering

Follow me on twitter for more posts like this

I was refactoring some legacy JavaScript recently and saw some things I needed to improve.

Here are the things I noticed with some descriptions.

Flattening arrays of objects to custom delimited arrays

I came across a number of arrays that contained flattened objects with custom delimiters.

// What was in the legacy code (wrong way to do this)
 ["myname", 30, "[!d]". "thesecondname", 30]

This was parsed out in for loops to detect each [!d] delimiter. This means the consumer has to understand the custom delimited format and/or assume there is a fixed index length to represent an object. You can store objects in an array and serialise them to json for passing around.

// standard way to do something like this
;[
  {
    name: 'myname',
    age: 30,
  },
  {
    name: 'thesecondname',
    age: 30,
  },
]

Pasting library code in to large domain logic files

I came across some instances of library code for handling date and number parsing pasted in to the middle of a large (5k+ lines) JavaScript file.

This makes it difficult to locate, change or remove later. Better to use npm these days or at least paste the code in to a separate file and load it manually that way. Much easier for the next developer to come along and remove it or change it.

Using strings as boolean flags

// Say you have some sort of settings object like this
settings:{
  NewSaleResetsSalesPerson: "Yes",
  SyncSavedOrders: "Yes"
}

// And now everytime you need to check a setting you have to check the string
if (Settings.FirstRun != "Yes"){...}

Use a boolean for these kinds of flags. If you need to display the boolean as a readable “Yes” somewhere in the UI you should apply that only in the UI.

// settings object using booleans
settings:{
  NewSaleResetsSalesPerson: true,
  SyncSavedOrders: true
}

// And now the value will be truthy and falsey as expected
if (!Settings.FirstRun){
  someUiElement.text("No")
}

Not using the replace method as regex

I noticed the replace method was used repeatedly to replace the same item. It seems like this is done to ensure all instances of the value are replaced. The JavaScript replace function uses regex. You need to specify that you want to replace globally.

// The same replace function is repeated here
if (Utils.HasString(Settings.VehicleName)) {
  if (strSettingsValue.lastIndexOf('Sedan') > 0) {
    strSettingsValue = strSettingsValue.replace('Sedan', Settings.VehicleName)
    strSettingsValue = strSettingsValue.replace('Sedan', Settings.VehicleName)
    strSettingsValue = strSettingsValue.replace('Sedan', Settings.VehicleName)
    strSettingsValue = strSettingsValue.replace('Sedan', Settings.VehicleName)
    strSettingsValue = strSettingsValue.replace('Sedan', Settings.VehicleName)
    strSettingsValue = strSettingsValue.replace('Sedan', Settings.VehicleName)
    strSettingsValue = strSettingsValue.replace('Sedan', Settings.VehicleName)
    strSettingsValue = strSettingsValue.replace('Sedan', Settings.VehicleName)
    strSettingsValue = strSettingsValue.replace('Sedan', Settings.VehicleName)
    strSettingsValue = strSettingsValue.replace('Sedan', Settings.VehicleName)
    strSettingsValue = strSettingsValue.replace('Sedan', Settings.VehicleName)
    strSettingsValue = strSettingsValue.replace('Sedan', Settings.VehicleName)
    strSettingsValue = strSettingsValue.replace('Sedan', Settings.VehicleName)
    strSettingsValue = strSettingsValue.replace('Sedan', Settings.VehicleName)
    Settings[row['Name']] = strSettingsValue
  }
}

// The equivelant with global replacement would be
if (Utils.HasString(Settings.VehicleName)) {
  if (strSettingsValue.lastIndexOf('Sedan') > 0) {
    strSettingsValue = strSettingsValue.replace(
      '/Sedan/g',
      Settings.VehicleName
    )

    Settings[row['Name']] = strSettingsValue
  }
}

Writing custom date time formatting code

It’s really difficult to get datetime parsing right. Especially for a multiple locale website.

Use a library like date-fns or moment instead of writing custom parsing code.

// date-fns is very light weight and can do some great formatting for you
var ampm = hours >= 12 ? 'pm' : 'am'
var minutes = minutes < 10 ? '0' + minutes : minutes

Overuse of alerts and error messages instead of input validation

I found there were lots of alerts and error messages for input. It can be a much better experience for the customer if they simply cannot enter bad data.

In this example, if they can only tick one item then maybe checkboxes are not the best UI element for this task. Consider a drop down or set of radio buttons.

// numberOfItems is essentially a count of checked checkboxes
if (numberOfItems > 2) {
  alert(
    'Operation can only be conducted on single items.\nUntick all except one.'
  )
}

Using boolean method input parameters

If you have a method that takes a boolean and operates differently based on the boolean, it’s difficult for the reader of the code to understand what the boolean is doing without reading the method source.

It’s better to just have two methods that have names that accurately describe what will happen when you call it.

// This is difficult to understand without knowing how Update works. In this case with true a form is cleared. With false it is not cleared before updating the UI.
MainPanel.Update(true)

// This provides more information about what will happen without having to read the Update method.
MainPanel.ClearAllFormElements()
MainPanel.UpdateFromServer()

If you see these JavaScript anti-patterns in your code, think about refactoring them to make it easier for the next developer.

Darragh ORiordan

Hi! I'm Darragh ORiordan.

I live and work in Sydney, Australia building supporting happy teams that create high quality software for the web.

I also make tools for busy developers. Have you ever spent a week getting your dev environment just right?

My DevShell tooling will save you 30+ hours configuring your dev environment with all the best modern tools. Get it here

https://darraghoriordan.gumroad.com/l/devshell


Read more articles like this one...

List of article summaries

#developer-experience

How engineers can help deliver software effectively

Delivery managers and team leads have the responsibility to deliver a software system via an engineering team.

Your customer wants every feature to work perfectly and they want it delivered yesterday. Your team wants to learn and grow.

It’s a tough role managing all the stakeholders and creators in a project.

Engineers can help drive great delivery by empathising with and supporting the delivery manager or leads in a project team.

#engineering

Engineering systems for consistency and impact

Your most impactful engineering is done before you write any code.

It’s important to have some systems around how you approach problems to make sure you’re consistent every time.

These are some of the techniques I use to make sure I’m covering as many angles as possible when doing my pre-coding engineering.

#engineering

How software engineers can avoid commoditisation

Engineers spend most of their learning time on technical implementation content. Things like new frameworks, languages or cloud platforms.

But turning solutions into code is a tiny part of what you do and it’s getting less valuable year by year.

As we’ve seen with “no-code” and tools like GitHub Copilot, the implementation part of our role is increasingly becoming commoditised.

You could generalise that the value an engineer brings to a team is their ability to analyse problems and synthesise context. The part of your role as an engineer that will never be replaced by “no-code” or AI is this high level cognition.

The true human aspect of being an engineer is working in a team and considering other people’s ideas, emotions and thoughts while solving these problems.

So shouldn’t you train these meta-cognition skills as much as you train the specific technologies?

Every engineer should spend time learning and applying general tools for thinking. These tools are applicable to almost all problems so the compounded payback on your invested time is huge.

Clearer thinking will amplify all the other skills you have and any frameworks or tools you learn will give you results for the rest of your career.

Like any skill, improving the way you think takes deliberate study and practice.

These are some of the tools and systems for thinking that I refer back to all the time.

#engineering

20 questions for a valuable code review

I recently had an interesting discussion around the value of doing code reviews and the value of mandatory code reviews.

I think code reviews are extremely valuable and should be done by most organisations and teams.

A valuable code review will

  • pass institutional knowledge around the org
  • help all engineers grow their skills
  • maintain quality in the face of all the other time pressures your team faces

But how do you keep a code review valuable?