-3

I have a piece of code where I need to assign a value to a variable, but base on condition, it could take other values. It can be done in multiple ways, but need to know which would be preferred in terms of optimization.

1. Assigning the default value in the last else

if (deviceInfo.pop) {
    this.forms = getPopFormModel();
    this.formMappings = getPopFormMapping(deviceInfo.wlMacs);
} else if (deviceInfo.mode === CONST.MODES.CN) {
    this.forms = getCnFormModel();
    this.formMappings = getCnFormMapping(deviceInfo.wlMacs);
} else {
    this.forms = getDnFormModel();
    this.formMappings = getDnFormMapping(deviceInfo.wlMacs);
}

2. Assigning default before checking for conditions

this.forms = getDnFormModel();
this.formMappings = getDnFormMapping(deviceInfo.wlMacs);

if (deviceInfo.pop) {
    this.forms = getPopFormModel();
    this.formMappings = getPopFormMapping(deviceInfo.wlMacs);
} else if (deviceInfo.mode === CONST.MODES.CN) {
    this.forms = getCnFormModel();
    this.formMappings = getCnFormMapping(deviceInfo.wlMacs);
}

It looks like assigning 1st version would be better since there is no chance of calling two functions. Would be helpful if someone can point to any references supporting the answer as well.

PS: Also consider a scenario where the called functions are simply returning objects without any additional functionality.

Thanks

Sachin Gupta
  • 103
  • 1
  • 1
    How many times does this code get executed (per second). If not at least 100000, speed differences are irrelevant. – Jan Doggen Sep 24 '20 at 08:12
  • Both of these sucks, I'd have put the function in a variable: `formFactory = getPopFormModel;` and then call all of them at the end, or use polymorphism to remove the if-statement here. – Lie Ryan Sep 27 '20 at 06:26

2 Answers2

5

When it's just about a couple of assignments, readability is easily 10 or 100 times more important than efficiency.

Assigning a variable unconditionally and then potentially assigning it again based on a condition trips your understanding and makes it look as if you don't know what you're doing. The else keyword expresses exactly what you want to do: "do this if none of the other conditions hold". Therefore, the threefold alternative should be expressed as a threefold if/else.

(When your code is in an inner loop that you've determined to be the bottleneck of a major module of functionality, efficiency may begin to trump readability. For everything else, don't bother.)

Kilian Foth
  • 107,706
  • 45
  • 295
  • 310
2

Performance is totally unimportant. Nobody cares. What makes the second code not bad but godawful is that it is misleading.

You assign a value, then overwrite it which looks pointless. Since people don’t write pointless code I have to figure out what your reason was. The only possible reason is that the two calls have some hidden side effect. So I look at these functions and look for some hidden side effect.

I waste lots of time, and all because you wrote stupid code.

gnasher729
  • 42,090
  • 4
  • 59
  • 119