Is something wrong with this switch?

Trying to get the icon to show up on the weather app. It just goes to the default. Here’s the switch:

switch (iconCode) {
case iconCode > 199 && iconCode < 233:
$(‘img’).attr(‘src’, ‘http://openweathermap.org/img/w/11d.png’);
break;
case iconCode > 299 && iconCode < 322:
$(‘img’).attr(‘src’, ‘http://openweathermap.org/img/w/09d.png’);
break;
case iconCode > 499 && iconCode < 532:
$(‘img’).attr(‘src’, ‘http://openweathermap.org/img/w/10d.png’);
break;
case iconCode > 599 && iconCode < 623:
$(‘img’).attr(‘src’, ‘http://openweathermap.org/img/w/13d.png’);
break;
case iconCode > 700 && iconCode < 782:
$(‘img’).attr(‘src’, ‘http://openweathermap.org/img/w/50d.png’);
break;
case iconCode == 800:
$(‘img’).attr(‘src’, ‘http://openweathermap.org/img/w/01d.png’);
break;
case iconCode == 801:
$(‘img’).attr(‘src’, ‘http://openweathermap.org/img/w/02d.png’);
break;
case iconCode == 802:
$(‘img’).attr(‘src’, ‘http://openweathermap.org/img/w/03d.png’);
break;
case iconCode == 803:
$(‘img’).attr(‘src’, ‘http://openweathermap.org/img/w/04d.png’);
break;
case iconCode == 804:
$(‘img’).attr(‘src’, ‘http://openweathermap.org/img/w/04d.png’);
break;
default:
$(‘img’).attr(‘src’, ‘http://openweathermap.org/img/w/01d.png’);
};

Each case in a switch checks for whether or not the value you provide is the same as the same as the expression provided:

switch(expression) {
  case value:
  // ...
}

The issue with your switch is that you are providing the results of comparisons as values, which evaluate to either true or false. As such, your switch will always return the default case because iconCode is never true or false.

Here is a shortened example of how it should be written:

switch(iconCode) {
  case '800':
    $('img').attr('src', 'http://openweathermap.org/img/w/01d.png');
    break;
  case '801':
    $('img').attr('src', 'http://openweathermap.org/img/w/02d.png');
    break;
  // ...
}

For the more complex cases where you are checking if iconCode is inside a range, you will have to figure out a ways to represent the results as a value if you are going to stick with using a switch in your code.

I hope that helps. :smile:

EDIT: typos and reformatted code.

In other words, a switch statement is not appropriate here, as javascript also doesn’t support case label ranges/patterns. Go for if-elseif, so you don’t end up copy pasting hundreds of lines and for everything from 800 onwards, you could use an object as a lookup table to make things even shorter.

I see. I just moved all my conditionals into an if, else if and it works now. Thanks, guys. Now I know.

well, one last thing. why won’t this toggle back to celsius? it’s like the else statement won’t run on click. if I run the conditions inside the else statement from the console, it works and I can click and change it to farenheit, but can’t click and change it back to celsius. I don’t get any errors. but nothing happens on click.

$("#deg").click(function() {
  if($("#deg").html("<a class='pointer'>C&deg</a>")){
    $("#temp").text(Math.round(faren));
    $("#deg").html("<a class='pointer'>F&deg</a>");
  }else {
    $("#temp").text(Math.round(celsius));
    $("#deg").html("<a class='pointer'>C&deg</a>");
  }   
});

Thanks. I had to take the degree symbol out of the span to get it the first condition to evaluate to true. I got it, now. Just have to style the app and it’s done. Thanks for the lessons, guys.

Here’s a suggestion. Remove this ugly syntax checking. Any time you’d update the html, this could potentially break.

The key thing to realize here is that the display can be either in C or F, nothing else. And you know which one it is when the page first loads. Meaning you don’t have to do any detection of what is displayed, just track it.

So for example, add set tracking variable on page load and toggle it each time the click handler is used. In the handler itself, just check the var. Eg.

let isCelsius = true; // initial unit
/.../
$("#deg").click(function() {
  if (isCelsius){
    $("#temp").text(Math.round(celsius));
    $("#deg").html("<a class='pointer'>C&deg</a>");
  } else {
    $("#temp").text(Math.round(faren));
    $("#deg").html("<a class='pointer'>F&deg</a>");
  }
  isCelsius = !isCelsius;
});

Plus, you could change the text directly by specifying one selector more, eg. $("#deg .pointer").text("C&deg");.

2 Likes

yeah, that definitely looks a lot better.