7

I have a piece of code that has a lot of if and else if. And I just thought now that in multiplication, true evaluates to 1 and false evaluates to 0. Would it be safe and easier to read (because it's shorter) to substitute :

if(!this._isFetched('studentInfoFetched')) { 
        tempAddedTime += 1;
        estimatedTimePerStudent += 0.04 + 0.2;
}
if(formInputValues.student_expiration){
        tempAddedTime += (!this._isFetched('studentExpirationFetched'))? 14 : 0;
        estimatedTimePerStudent += 1; 
}  

for :

    tempAddedTime += 1 * (!this._isFetched('studentInfoFetched')) + 14 * (!this._isFetched('studentExpirationFetched')) * (formInputValues.student_expiration);
    estimatedTimePerStudent += 0.24 * (!this._isFetched('studentInfoFetched')) + 1 * (formInputValues.student_expiration);

Note :_isFetched returns a bool. And this is just an example, for other cases I have a lot more if's so it would be saving me more lines.

9
  • 2
    I would say it's your call. I personally would leave it and go with the first, simply because it is easier to update, and I find the format easier to read because of less horizontal length. Nov 4, 2015 at 20:15
  • 1
    Would it be safe probably (depending on your definition of "safe"), and easier to read definitely not. Nov 4, 2015 at 20:15
  • 5
    the 2nd one is confusing as all get out... MAYBE space it out more, but in general, avoid being clever.
    – dandavis
    Nov 4, 2015 at 20:16
  • 2
    It's not a good idea to do this. Interpreters/compilers should make the necessary optimizations for you. All you are doing is making your code hard to read. Also, this question is probably a better fit for CodeReview.
    – Dave
    Nov 4, 2015 at 20:16
  • Also, your second example calls this._isFetched('studentInfoFetched') twice which, (depending on what is involved in a call to _isFetched) is less efficient. And that's assuming that the value returned will be the same both times! Nov 4, 2015 at 20:17

2 Answers 2

10

No, the ifs - version is better.

Reasons:

  • It's much more readable: expressions are shorter, and the lines are not too long. For example, I see a horizontal scrollbar on my screen for your multiplication expressions, while I don't have to scroll in the if-snippet :)

  • It's faster because you avoid the multiplication.

  • It's even faster because you avoid calling this._isFetched('studentInfoFetched') twice.

  • ifs semantically define program flow, while the multiplication is semantically a mathematical expression which is used to fake the program flow in this case. With ifs, statements are grouped by condition, and you see at a glance what happens if a certain condition is met.

Then, consider that you have to create two more if clauses. The multiplication would become totally unmaintainable.

4
  • Thanks for the detailed explanation. I completely agree but couldn't think of these myself. I guess my problem was having 300 lines of If's to change the value of 3 variables. but I guess it's clearer and faster with If's
    – Anthony N.
    Nov 4, 2015 at 21:58
  • 2
    "It's faster because you avoid the multiplication." This is not true. Google branchless Programming
    – Niton
    Sep 1, 2020 at 9:50
  • @Niton, please create and share a benchmark before claiming that multiple multiplications, including floating-point numbers, is faster than two ifs. Sep 1, 2020 at 10:53
  • @AlexShesterov in general branchless can do a well job in increasing performance BUT it is indeed very dependend of the usecase : language, used compiler, used processor. You can do a look under the hood and take a look at what the compiler produces but in general branchless programming leads to better performance, i didnt do the benchmarks but others did. Again it is possible (and not more) that using if is faster in this case.
    – Niton
    Sep 12, 2020 at 23:21
1

The code should be easy to read, quick to understand, and fast to change.

Better than comments that go out off time is clear variable names, although they're good for a general desctiption of why. Name constant (ie. what is 0.04 + 0.2??), and name expressions for brevity in context (also avoids unnecessary function calls).

// Estimate process time
const infoFetched = this._isFetched('studentInfoFetched')
const infoFetchTime = 0.04 + 0.2
const canExpire = formInputValues.student_expiration
const expirationFetched = this._isFetched('studentExpirationFetched')
const expirationFetchTime = 14

if (!infoFetched) tempAddedTime += 1
if (hasExpired && !expirationFetched) tempAddedTime += expirationFetchTime

if (!infoFetched) estimatedTimePerStudent += fetchTime
if (hasExpired) estimatedTimePerStudent += 1

I usually like to multiply booleans as toggles, although in this case, the if's might be a little bit easier to read, understand, and change;

tempAddedTime +=
    !infoFetched* 1 +
    (hasExpired && !expirationFetched)* expirationFetchTime

estimatedTimePerStudent +=
    !infoFetched* fetchTime +
    hasExpired* 1

Not the best example, would probably be quite different if I had access/knowledge about what it itended to do / the source

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.