CODEWITHSUNDEEP
javascriptif-statementisnullorempty
Elvira
Elvira

Reputation: 1430

A better way for checking on empty/null/undefined

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)?

Upvotes: 0

Views: 180

Answers (4)

Piero Nicolli
Piero Nicolli

Reputation: 196

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;
  }
});

Upvotes: 1

brunnerh
brunnerh

Reputation: 184271

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;
});

Upvotes: 1

Stephan T.
Stephan T.

Reputation: 6074

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);
}

Upvotes: 0

dloeda
dloeda

Reputation: 1548

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:

Falsy/Truthy JS Table

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;
  }
});

Upvotes: 2

Related Questions