How to reduce the size of this function that contains multiple if?

Asked

Viewed 56 times

1

I wanted to think of a logic that would decrease the size of this code, because it ends up repeating itself.

It is an experience calculator for a game. You set your current level and the level you want to reach. The game has normal level ranging from 1 to 399 and the master level from 400 to 1300.

Basically I need to check that both the current level and the desired level are in the correct category (whether it is in normal or master).

Notice that both the if of currentLevel as to the desiredLevel are practically identical, only changes even if one is for the current and the other is for the desired.

Pay no attention to array have more variables, I only brought the part that interests me here. The function has more than that.

let checkfields = [currentLevel, desiredLevel, currentExp, expSec, hoursDay]

            function checkFieldInsert(normalInitialLevel, masterInitialLevel){
                for (let i = 0; i <= checkfields.length; i++){
                    if (checkfields[i] == currentLevel){
                        if (levelMaster.checked && parseInt(checkfields[i].value) < 400){
                            alert(`this ${checkfields[i].value} is not master level`)
                            break
                        } 
                        else if (levelNormal.checked && parseInt(checkfields[i].value) >= 400){
                            alert(`this ${checkfields[i].value} is not normal level`)
                            break
                        }
                    } else if (checkfields[i] == desiredLevel){
                        if (levelMaster.checked && parseInt(checkfields[i].value) < 400){
                            alert(`this ${checkfields[i].value} is not master level`)
                            break
                        } 
                        else if (levelNormal.checked && parseInt(checkfields[i].value) >= 400){
                            alert(`this ${checkfields[i].value} is not normal level`)
                            break
                        }
                    }
        }
      }  

1 answer

2


The two main conditions are almost identical, only one subexpression is different and what it performs when it enters each is identical, so it really doesn’t make sense to have two conditions. The code has an identical pattern except for one detail, so making it stay just once is easy and just changes the subexpression, so it enters being a subexpression being true OR the other being in the other if being true as well. I put in parentheses because it needs to be done before the rest.

I also drew the comparison with true because he already expects one true does not make sense to make this comparison, what already reduces a little the size of the code.

You can reduce it a little bit more but I think it doesn’t make up for it, it starts to get crowded and confused.

I can’t guarantee you’ll do what you want, I just cut the code without context.

function checkFieldInsert(normalInitialLevel, masterInitialLevel){
    for (let i = 0; i <= checkfields.length; i++){
        if ((checkfields[i] == currentLevel || checkfields[i] == desiredLevel) && levelNormal.checked && parseInt(checkfields[i].value) >= 400 || levelMaster.checked && parseInt(checkfields[i].value) < 400){
            if (levelMaster.checked){
                alert(`Level ${checkfields[i].value} is not master level`)
                break
            } else if (levelNormal.checked){
                alert(`Level ${checkfields[i].value} is not normal level`)
                break
            }
        }
    }   
}

I put in the Github for future reference.

  • Wow, it worked! I didn’t know I could use parentheses like that... I had tried to do exactly the same thing you did, but without the parentheses and it didn’t work out. Thank you very much.

Browser other questions tagged

You are not signed in. Login or sign up in order to post.