JavaScript

WTF code, defects, and the principle of least astonishment

We have recently introduced two defects, while trying to improve old, obscure code. Of course we are to blame for not being more paranoid and not researching more why the code was there. But if the code was clear to start with, if it followed the least astonishment principle, the errors wouldn’t have happened. In both cases the original code provoked the “WTF?!” reaction in the reader.

Case 1: Ignore optional property when missing

We had a React component with two ways to control it being shown or not: either by setting showInitially or by setting show. This piece of code was intended to handle the latter case:

onSetProperties(props) {
  if (!props.show) return;
  this.setState({ show: props.show })
}

Why the if was necessary was quite unclear.

This would have been much more explicit and thus better:

onSetProperties(props) {
  if (_.isUndefined(props.show)) return;
  this.setState({ show: props.show })
}

Even better, (which we eventually did), the component would be refactored to support just a single way of being displayed/hidden, that fit all use cases.

Case 2: Returning an “error data” when resource not found

We had this function:

function getUserAccountData(userId) {
  return httpRequest(`/service/account/${userId}`).then(response => {
    if ([200, 404].includes(response.httpStatus)) return JSON.parse(response.body);
    else throw new RequestFailedError(response);
  })
}

So, when the user did not exist, it returned something like this:

{
     error_code: 20
     reference: '5ee7c248-6865-4458-d06a-21e34c10456d'
}

instead of the normal account data. That was contrary to all the other services, that threw either the generic RequestFailedError or, where needed, the more specific NotFoundError. And it made it more difficult for a new usage that actually needed to treat 404 as an error.

So, not to surprise the reader, we could do one of these upon 404:

1. Return null and, preferably, rename the function to indicate that it can return that (contrary to most other ones) to e.g. getUserAccountDataIfExists.
2. Throw NotFoundError and let the piece of code that accepts 404 to catch it and turn into null or whatever it needs.

Conclusion

Try to be explicit and as clear as possible, follow the least astonishment principle – i.e. make sure that your code behaves in the same way as similar code elsewhere in the app.

Published on Web Code Geeks with permission by Jakub Holy, partner at our WCG program. See the original article here: WTF code, defects, and the principle of least astonishment

Opinions expressed by Web Code Geeks contributors are their own.

Jakub Holy

Jakub is an experienced Java[EE] developer working for a lean & agile consultancy in Norway. He is interested in code quality, developer productivity, testing, and in how to make projects succeed.
Subscribe
Notify of
guest

This site uses Akismet to reduce spam. Learn how your comment data is processed.

0 Comments
Oldest
Newest Most Voted
Inline Feedbacks
View all comments
Back to top button