Is something wrong with this switch?

Is something wrong with this switch?
0.0 0

#1

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’);
};


#2

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.


#3

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.


#4

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


#5

Actually, you can use the switch statement here with your existing conditions. All you have to do is change the expression value (the part inside the parentheses) to true. Then, this will get matched against each case clause.

switch(true) {
 .
 . 
 .
}


#6

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>");
  }   
});

#7

I’ve edited your post for readability. When you enter a code block into the forum, precede it with a line of three backticks and follow it with a line of three backticks to make easier to read. See this post to find the backtick on your keyboard. The “preformatted text” tool in the editor (</>) will also add backticks around text.


#8

When you write

$("#deg").html("<a class='pointer'>C&deg</a>")

you update the element with id=“deg” to <a class='pointer'>C&deg</a>. This is a non-empty string, so all non-empty strings have a truthy value, so it is as if you wrote:

if(true) {

Now if you wrote:

if($("#deg").html() === "<a class='pointer'>C&deg</a>") {

then you would be comparing the current contents of the element with id=“deg” to “<a class='pointer'>C&deg</a>”.


#9

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.


#10

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");.


#11

yeah, that definitely looks a lot better.


#12

Adding to @lynxlynxlynx’s great suggestion. You could make the code a little more DRY with:

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