CODEWITHSUNDEEP
javascript
alex
alex

Reputation: 7601

How to simplify the following if statements and ternary operators?

The following code is inside a loop which loops through the fields of a form. If isV1User is true, then the field is disabled. If the user has customSetting then don't disable the field. If the user doesn't have it, disable it. 

if (field.name === 'themeColor') {
  if (this.isV1User) {
    field.disabled = false
  } else {
    field.disabled = this.user.customSetting
      ? !this.user.customSetting.themePicker
      : false
  }
}

How to simplify or at least remove the nesting of this code?

Upvotes: 0

Views: 59

Answers (6)

Charles
Charles

Reputation: 4352

The way that you have structured your code, it can be minimized to the following equivalent code. Please note that if this.user.customSetting.themePicker is guaranteed to always be true when this.user.customSetting is true, you can set field.disabled = true in a single if statement where the conditional is field.name == 'themeColor':

if (field.name == 'themeColor' && this.user.customSetting) {
 field.disabled = !this.user.customSetting.themePicker;
} else if (field.name == 'themeColor') {
 field.disabled = false;
}

Or even the following switch statement, depending on how you want your code structured. They both are the same.

switch (field.name) {
  case 'themeColor':
    if (this.user.customSetting) {
      field.disabled = !this.user.customSetting.themePicker;
    }
    break;
  default:
    field.disabled = false;
}

Most of these answers break the cardinal rule of ternary statement readability. If your goal is simplistic readability, breaking it down to a simple if/else if statement will do. If you are trying to minimize the code as much as possible, and don't care if it's unmaintainable/hard to read, you should simplify it to a recursive ternary statement. Personally, I find that long ternary statements do not provide significant space-saving, hamper readability, and should be avoided in cases where they are not extremely simple (ie: var x = statement? 1 : 0;)

Upvotes: 1

brk
brk

Reputation: 50291

You can do like this

    "themeColor"===field.name && (field.disabled=this.isV1User?!1:
this.user.customSetting?!this.user.customSetting.themePicker:!1);

Upvotes: 1

Artem Kolodko
Artem Kolodko

Reputation: 409

Try this

if (field.name === 'themeColor') {
   field.disabled = this.isV1User ? true : this.user.customSetting ? !this.user.customSetting.themePicker : false;
}

Upvotes: 0

RemcoGerlich
RemcoGerlich

Reputation: 31250

This isn't really a Stack Overflow question, would fit better on Code Review I guess.

There's only one set of circumstances in which it needs to be true, it seems, so maybe like this?

if (field.name === 'themeColor') {
    field.disabled = (
        !this.isV1User &&
        this.user.customSetting && !this.user.customSetting.themePicker);
}

The first if is still needed because other fields should remain unchanged (I assume).

Upvotes: 1

Ivan
Ivan

Reputation: 895

if (field.name === 'themeColor') {
    field.disabled = this.user.customSetting && !this.isV1User ?
    !this.user.customSetting.themePicker : false;
}

Upvotes: 1

Justinas
Justinas

Reputation: 43451

Move every if condition to ternary:

if (field.name === 'themeColor') {
    field.disabled = !this.isV1User && this.user.customSetting && !this.user.customSetting.themePicker;
}

Upvotes: 3

Related Questions