If statement not being entered in for loop

Question

I have a geojson file with earthquake point features that need to be styled based on the magnitude of the quake. The switch statement works correctly but it's a lot of code that I think could be done neater:

switch (feature.properties.mag){
            case 1.0 : return layer.setRadius(5);
            case 1.1 : return layer.setRadius(5);
            case 1.2 : return layer.setRadius(5);
            case 1.3 : return layer.setRadius(5);
            case 1.4 : return layer.setRadius(5);
            case 1.5 : return layer.setRadius(5);
            case 1.6 : return layer.setRadius(5);
            case 1.7 : return layer.setRadius(5);
            case 1.8 : return layer.setRadius(5);
            case 1.9 : return layer.setRadius(5);
            case 2.0 : return layer.setRadius(8);
            case 2.1 : return layer.setRadius(8);
            case 2.2 : return layer.setRadius(8);
            case 2.3 : return layer.setRadius(8);
            case 2.4 : return layer.setRadius(8);
            case 2.5 : return layer.setRadius(8);
            case 2.6 : return layer.setRadius(8);
            case 2.7 : return layer.setRadius(8);
            case 2.8 : return layer.setRadius(8);
            case 2.9 : return layer.setRadius(8);
            case 3.0 : return layer.setRadius(10);
            case 3.1 : return layer.setRadius(10);
            case 3.2 : return layer.setRadius(10);
            case 3.3 : return layer.setRadius(10);
            case 3.4 : return layer.setRadius(10);
            case 3.5 : return layer.setRadius(10);
            case 3.6 : return layer.setRadius(10);
            case 3.7 : return layer.setRadius(10);
            case 3.8 : return layer.setRadius(10);
            case 3.9 : return layer.setRadius(10);
            case 4.0 : return layer.setRadius(15); 
            case 4.1 : return layer.setRadius(15);
            case 4.2 : return layer.setRadius(15);
            case 4.3 : return layer.setRadius(15);
            case 4.4 : return layer.setRadius(15);
            case 4.5 : return layer.setRadius(15);
            case 4.6 : return layer.setRadius(15);
            case 4.7 : return layer.setRadius(15);
            case 4.8 : return layer.setRadius(15);
            case 4.9 : return layer.setRadius(15);
            case 5.0 : return layer.setRadius(20);
            default: layer.setRadius(8);
        }

I think the for loop might be caught up on the conditional statement because all the points are styled the same and not with the else value:

for (var i=0; i<feature.properties.length; i++){
            if (1.0 <= feature.properties.mag < 2.0){
                 return layer.setRadius(5);
            }

            else if (2.0 <= feature.properties.mag < 3.0){
                 return layer.setRadius(8);
            }

            else if (3.0 <= feature.properties.mag < 4.0){
                 return layer.setRadius(10);
            }

            else if (4.0 <= feature.properties.mag < 5.0){
                 return layer.setRadius(15);
            }

            else{
                 return layer.setRadius(2);
            }
            }

is it possible for `feature.properties.mag` to be `1.05`, etc between those case values? If so, the `if` is definitely the way to go — mswieboda, Jun 12 '15 at 14:45
you can group switch statements, see : http://stackoverflow.com/questions/13207927/switch-statement-multiple-cases-in-javascript — Hacketo, Jun 12 '15 at 14:46
What's the point of the for loop? you're not iterating over anything.. — winhowes, Jun 12 '15 at 14:47
There is no point to the for loop, new to java was just overthinking it. The magnitude values only have 1 significant digit, but if is still the way to go. — user2166059, Jun 12 '15 at 14:58

Wee Tai · Accepted Answer · 2015-06-12T14:49:33.807

3

First, your conditionals are not right. Try them like this setting a lower and upper bound.

if (feature.properties.mag >= 1.0 && feature.properties.mag < 2.0){
    return layer.setRadius(5);
}
// the rest similar to the above

Unless there is more to feature.properties we aren't seeing, the for-loop is unnecessary.

edited Jun 12 '15 at 14:49

answered Jun 12 '15 at 14:47

Wee Tai

123
6

Should probably get rid of the `for` loop also since it doesn't do anything. – jfriend00 Jun 12 '15 at 14:48
Agreed. For loop isn't needed... unless there is more to `feature.properties` we aren't seeing – Wee Tai Jun 12 '15 at 14:51
It's worth explaining what OP's code does. It evaluates the left inequality (e.g., `1.0 <= feature.properties.mag`) and obtains a `true` or `false` value. That value is then converted to an integer value (either 1 for `true` or 0 for `false`) to do the second comparison. – Ted Hopp Jun 12 '15 at 14:53
Cool! BTW @TedHopp made a good comment about the inequalities. – Wee Tai Jun 12 '15 at 14:59

Kudin · Answer 2 · 2015-06-12T14:59:12.980

0

Why not use Math.floor() on feature.properties.mag?

switch (Math.floor(feature.properties.mag)){
       case 1.0 : return layer.setRadius(5);
       case 3.0 : return layer.setRadius(10);
       case 4.0 : return layer.setRadius(15); 
       case 5.0 : return layer.setRadius(20);
       default: return layer.setRadius(8);
}

Switch is a replacement for long if-elseif chains

edited Jun 12 '15 at 14:59

answered Jun 12 '15 at 14:53

Kudin

456
4
7

You need a `return` in the `default` case. – Ted Hopp Jun 12 '15 at 14:56
Edited answer, sorry) – Kudin Jun 12 '15 at 14:59
Was not aware of the Math.floor() function, that code looks a lot cleaner. Thanks! – user2166059 Jun 12 '15 at 15:01

If statement not being entered in for loop

2 Answers2