A better way for checking on empty/null/undefined

Question

I'm wondering if I can upgrade my code. I have the following steps: when a title is not found, a book is not available. When some parts of the metadata of a book are not found, the book is still available, but the requested data should be returned as unknown.

var notAvailableMSG = process.env.notAvailableMSG;
var unknownMSG = process.env.unknownMSG;
var bookInfo = [];

// if title is not found = book is not available.
if(bookInfo.title == '' || bookInfo.title == undefined ){
  bookInfo.isbn = isbn;
  bookInfo.title = notAvailableMSG;
  bookInfo.author = notAvailableMSG;
  bookInfo.publisher = notAvailableMSG;
  bookInfo.price = notAvailableMSG;
  bookInfo.currency = notAvailableMSG;
  bookInfo.url = notAvailableMSG;
  bookInfo.releasedate = notAvailableMSG;
  bookInfo.sellable = notAvailableMSG;
}
// Check for other empty values
if(bookInfo.author == '' || bookInfo.author == undefined){
  bookInfo.author = unknownMSG;
}
if(bookInfo.publisher == '' || publisher == undefined){
  bookInfo.publisher = unknownMSG;
}
if(bookInfo.price == '' || bookInfo.price == undefined){
  bookInfo.price = unknownMSG;
}
if(bookInfo.releasedate == '' || bookInfo.releasedate == undefined){
  bookInfo.releasedate = unknownMSG;
}

Can this code be written in a better way to let this piece of code run faster (make it more readable)?

"Running faster" and "more readable" are separate things. And usually you should primarily care about the latter. — H.B., Mar 27 '18 at 08:49
Looks like `var bookInfo = [];` should be `var bookInfo = {};` — Andy, Mar 27 '18 at 08:52

dloeda · Answer 1 · 2018-03-27T09:01:03.453

2

I think the easiest way to check if a value is empty is checking if it's falsy. It isn't the fastest code, but in my opinion is easier to read:

if(!bookInfo.author){
  bookInfo.author = unknownMSG;
}
if(!bookInfo.publisher){
  bookInfo.publisher = unknownMSG;
}
if(!bookInfo.price && bookInfo.price !== 0){
  bookInfo.price = unknownMSG;
}
if(!bookInfo.releasedate){
  bookInfo.releasedate = unknownMSG;
}

You can check this post to see when a value is falsy in JS:

EDIT

As H.B. said you can use a function to check each key of the book. Also you can use Object.keys if you want to check each key of the object:

['author', 'publisher', 'price', 'releasedate', /* ... */].forEach(key => {
  if (bookInfo[key] === undefined || bookInfo[key] === null || bookInfo[key] === '') {
    bookInfo[key] = unknownMSG;
  }
});

edited Mar 27 '18 at 09:01

answered Mar 27 '18 at 08:53

dloeda

1,516
15
23

You could also remove those curly braces and have the statement on the same line as it's conditional. Might make it neater. – Andy Mar 27 '18 at 08:57
Would not recommend this. If the book is free (0), the price will be unknown. Should just refactor the thing into a function. – H.B. Mar 27 '18 at 08:59
I think the same, but there's a discussion over that braces in the big JS companies. It depends on the style guide of the company (Google, Airbnb, etc) – dloeda Mar 27 '18 at 08:59
Nice comment @H.B. – dloeda Mar 27 '18 at 09:00

H.B. · Answer 2 · 2018-03-27T09:06:10.970

Another possible refactoring:

const checkUnknown = prop => {
  if(bookInfo[prop] == '' || bookInfo[prop] == undefined){
    bookInfo[prop] = unknownMSG;
  }
}
checkUnknown('author');
checkUnknown('publisher');
checkUnknown('price');
checkUnknown('releasedate');

More fancy, taking title into account:

const checkUnknown = (prop, customCallback) => {
  if (bookInfo[prop] == '' || bookInfo[prop] == undefined) {
    if (customCallback)
      customCallback();
    else
      bookInfo[prop] = unknownMSG;
  }
}
checkUnknown('author');
checkUnknown('publisher');
checkUnknown('price');
checkUnknown('releasedate');
checkUnknown('title', () => {
  bookInfo.isbn = isbn;
  bookInfo.title = notAvailableMSG;
  bookInfo.author = notAvailableMSG;
  bookInfo.publisher = notAvailableMSG;
  bookInfo.price = notAvailableMSG;
  bookInfo.currency = notAvailableMSG;
  bookInfo.url = notAvailableMSG;
  bookInfo.releasedate = notAvailableMSG;
  bookInfo.sellable = notAvailableMSG;
});

I like this approach too, thank you for your answer! I appreciate it. — Elvira, Mar 27 '18 at 10:45

Piero Nicolli · Accepted Answer · 2018-03-27T10:50:16.030

1

When I have long lists of properties, I prefer avoiding repeated code or repeated ifs, if possible.

In this kind of scenarios I would go with something like:

var properties = [
  'author',
  'publisher',
  'price',
  'currency',
  'url',
  'releasedate',
  'sellable'
];

if(!bookInfo.title) {
  bookInfo.isbn = isbn;
  bookInfo.title = notAvailableMSG;
  properties.map(function(prop) {
    bookInfo[prop] = notAvailableMSG;
  });
}

// Check for other empty values
properties.map(function(prop) {
  if(!bookInfo[prop]) {
    bookInfo[prop] = unknownMSG;
  }
});

edited Mar 27 '18 at 10:50

answered Mar 27 '18 at 09:10

Piero Nicolli

196
1
5

I'm going for this approach :). You were missing a ); at the end of the properties.map. – Elvira Mar 27 '18 at 10:43

score 0 · Answer 4 · answered Mar 27 '18 at 09:01

Can't you just do this at all in the one if clause?

var notAvailableMSG = process.env.notAvailableMSG;
var unknownMSG = process.env.unknownMSG;
var bookInfo = {};

// if title is not found = book is not available.
if(bookInfo.title == '' || bookInfo.title == undefined ){
  bookInfo.isbn = isbn;
  bookInfo.title = ((!notAvailableMSG) ? unknownMSG : notAvailableMSG);
  bookInfo.author = ((!notAvailableMSG) ? unknownMSG : notAvailableMSG);
  bookInfo.publisher = ((!notAvailableMSG) ? unknownMSG : notAvailableMSG);
  bookInfo.price = ((!notAvailableMSG && bookInfo.price != 0) ? unknownMSG : notAvailableMSG);
  bookInfo.currency = ((!notAvailableMSG) ? unknownMSG : notAvailableMSG);
  bookInfo.url = ((!notAvailableMSG) ? unknownMSG : notAvailableMSG);
  bookInfo.releasedate = ((!notAvailableMSG) ? unknownMSG : notAvailableMSG);
  bookInfo.sellable = ((!notAvailableMSG) ? unknownMSG : notAvailableMSG);
}

Thank you for your answer and time. It is indeed possible to do it in one if. However I don' t really like the readability of this one. — Elvira, Mar 27 '18 at 10:47

A better way for checking on empty/null/undefined

4 Answers4