How to improve my code (many If’s and Else’s)

Asked

Viewed 1,008 times

3

I have a function to search a json locally and change the data of some Fields. however, I do not want to test if the field is with the status code and depending I change with the correct value for visualization in the frontend.

following example:

  $http.get("http://localteste:18888/tess").success(function (dados) {


         local_locations = $.map(dados, function (dev) {
            //Verifica Status da Bateria

            if (dev.status) {
                if (dev.status <= 10)
                    dev.status= "Muito Baixo";
               else if (dev.status<= 25)
                    dev.status = "Baixo";;
               else if ((dev.status== 50) || (dev.status>= 25))
                    dev.status= "Medio";;
              else  if ((dev.status== 75) || (dev.status>= 50))
                    dev.status= "Alto";;
               else if ((dev.status== 100) || (dev.status> 75))
                    dev.batUseBattery = "Muito alto";
       }
 });

That would be about right, but there’s a lot of tests like that. How could I improve?

javascript angularjs

  • What’s the point of reducing the number of if? Is it just because of reading? Regarding algorithmic complexity there is no problem.

3 answers

10


One of the things I didn’t understand in your code is why you’re using two comparisons by if. Supposedly one test in each is enough, because if any of the comparisons is true, you won’t get to the next If (as already stated in @Wakim’s reply):

// O if externo pode ser removido se você tem certeza de ser numerico
   if (dev.status <= 10) {
      dev.batUseBattery = "Muito Baixo";
   } else if (dev.status <= 25) {
      dev.batUseBattery = "Baixo";
   } else if (dev.status <= 50) {
      dev.batUseBattery = "Medio";
   } else if (dev.status <= 75) {
      dev.batUseBattery = "Alto";
   } else { // não precisa comparar nada aqui, afinal ja é maior que 75
      dev.batUseBattery = "Muito alto";
   }

Another alternative, if it has many values, is the use of a loop and a array to test the tracks (but in the amount of options in your example, I would if...else even).

dev.batUseBattery = "Muito baixo";
var niveis = [
    [10,'Baixo'], // Maior que 10 é "Baixo"
    [25,'Medio'], // Maior que 25 é "Medio" etc...
    [50,'Alto'],
    [75,'Muito alto'],
];
for (var i = 0; i < niveis.length; i++) {
   if( dev.Status > niveis[i][0] ) {
       dev.batUseBattery = niveis[i][1];
   }
}

Run your tests on JS Fiddle.

  • 1

    Pretty cool this second way, +1.

  • @Wakim the first, which is "almost a clone" of his (properly "upada"), I still think better for a few options, the second I left thinking of a case that the person had a situation with much more options.

  • 1

    I even thought of making a map to index with the status (if you didn’t have the range of 10 it was simple), but yours is much more readable.

  • Caraca. Sent well! The second option would be unfeasible to deploy now. But thank you so much for the remark! : D

  • 1

    Just out of curiosity, what is the cost-benefit ratio between the two options? What would be less "costly" for the browser to perform, Ifs or FOR with internal IF + ARRAY ?

  • @Fernando This kind of thing will depend a lot on the implementation of the browser, to know for sure only one benchmark even. Probably the loop will have a higher consumption as it performs the same amount of ifs of the top code plus the loop. But remember that we are talking about such negligible differences in this case, that it pays to think more about legibility than about over-optimization.

  • Exactly, I believe to be more readable the Ifs than a for. Mainly for rule keeping. Another thing @Bacco, it doesn’t work for "Low" Level... Try putting Status = 9 on your JS Fiddle...

  • @Fernando bass is 11 to 25, according to the author’s original code. I changed the if order in case he needs to check if batUseBattery is null (there he can put an IF to not even enter the loop). The array is tested for more than 10, more than 25 etc. Anyway, taking into account your consideration, I edited and put comments in the code to make it clearer.

  • Misconception of mine... I interpreted the reading of the array as an association between values and levels. You’re right @Bacco.

  • @Fernando de qq forma I edited and put comments in the code explaining, pq de facto the criterion I used could induce error.

  • To see how legibility is subjective, I find the technique of lookup more readable than the sausage of if, at least if you have sufficient options to compensate :)

Show 6 more comments

4

You can reduce these if's checking only the upper limit of the range, and taking advantage of the order in which conditions are assessed to fit the value into the correct range.

Be careful because if dev.status for 0, it will not classify correctly.

if ((typeof dev.status) !== 'undefined') {
    if (dev.status <= 10)
        dev.batUseBattery= "Muito Baixo";
    else if (dev.status <= 25)
        dev.batUseBattery = "Baixo";
    else if (dev.status <= 50)
        dev.batUseBattery= "Medio";
    else  if (dev.status <= 75)
        dev.batUseBattery= "Alto";
    else if (dev.status <= 100)
        dev.batUseBattery = "Muito alto";
}

I made a Jsfiddle so you can see the result.

2

I don’t think you have much to do. You can remove Elses and take breaks in Ifs but honestly I don’t see any organizational or optimization gains.

if ((typeof dev.status) !== 'undefined') {
  if (dev.status <= 10)
    dev.batUseBattery= "Muito Baixo";
  if (dev.status > 10 && dev.status <= 25)
    dev.batUseBattery = "Baixo";
  if (dev.status > 25 && dev.status <= 50)
    dev.batUseBattery= "Medio";
  if (dev.status > 50 && dev.status <= 75)
    dev.batUseBattery= "Alto";
  if (dev.status > 75)
    dev.batUseBattery = "Muito alto";
}

With switch does not give because are intervals and not exact values.

Browser other questions tagged javascript angularjs

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