CODEWITHSUNDEEP
if-statementcoffeescript
Benjamin Welch
Benjamin Welch

Reputation: 317

Why won't my CoffeeScript if/else statement work?

I've been trying to update a total price when someone changes the select option. Here is the select element I'm using:

<select id="payment_talks_purchased" name="payment[talks_purchased]">
  <option value="1">One</option>
  <option value="2">Three</option>
</select>

This is the jQuery I'm using:

jQuery(document).ready(function() {
  var price = $(".total-price span.price")
  var save = $(".savings")
  $("#payment_talks_purchased").change(function() {
    var selection = $("#payment_talks_purchased").val()
    if (selection == 2) {
      price.html("$12");
      save.css("visibility", "visible");
    } else if (selection == 1) {
      price.html("$5");
      save.css("visibility", "hidden");
    }
  });  
});

It works perfectly. It changes the price to $12 and shows the discount message. If I change the select option back to One/1, it changes the text back to $5 and removes the discount message.

I converted this to CoffeeScript but it only works when I make the first change. The price is updated. However, when I try to change it back to option 1, it doesn't update.

jQuery ->
  price = $(".total-price span.price")
  save = $(".savings")
  select = $("#payment_talks_purchased")
  select.change ->
    selection = select.val()
    if selection = 2
      price.html "$12"
      return save.css "visibility", "visible"
    else if selection = 1
      price.html "$5"
      return save.css "visibility", "hidden"

I've been working on this for hours and am at my wits end. Any help would be greatly appreciated.

Upvotes: 15

Views: 40352

Answers (3)

ppalmeida
ppalmeida

Reputation: 2954

Here is my .50 cents. Take in consideration 2 things: its just my simple opinion and it can be not the best answer of the world.

a) If you already have a return inside IF statement, no need ELSE IF

jQuery ->
price = $(".total-price span.price")
save = $(".savings")
select = $("#payment_talks_purchased")
select.change ->
    selection = select.val()
    if selection == '2'
        price.html "$12"
        // Since you return here, you dont need any other "else if"
        return save.css "visibility", "visible"

    price.html "$5"
    return save.css "visibility", "hidden"

And no, IMHO, puts the ELSE IF doesnt improve the readability. Return is a return. Period. It is simple as that.

jQuery ->
price = $(".total-price span.price")
save = $(".savings")
select = $("#payment_talks_purchased")
select.change ->
    selection = select.val()
    // "ternary" coffee version (if then else)
    price.html if selection == '2' then "$12" else "$5")
    save.css "visibility" (if selection == '2' then "visible" else "hidden")

But, better than all is get rid of IF, ELSE, SWITCH and all those craps. Think OOP and your code can start getting better. A starting point could be:

options = [
             {price: '$12', visible:"visible"}, 
             {price: '$5', visible:"hidden"}
          ];
jQuery ->
    price = $(".total-price span.price")
    save = $(".savings")
    select = $("#payment_talks_purchased")
    select.change ->
              // If the val of your select was 0 and 1, you wouldnt need the (-1) part
      selection = parseInt(select.val) -1
              price.html options[selection].price
              save.css "visibility" options[selection].visible

So, this is it. Almost the same code, but with a better implementation (imho). Thank you.

Upvotes: 1

mu is too short
mu is too short

Reputation: 434625

Your selection = 1 inside your if statements is (still) an assignment in CoffeeScript, you need to use == for comparison. Try this:

jQuery ->
  price = $(".total-price span.price")
  save = $(".savings")
  select = $("#payment_talks_purchased")
  select.change ->
    selection = select.val()
    if selection == '2'
      price.html "$12"
      return save.css "visibility", "visible"
    else if selection == '1'
      price.html "$5"
      return save.css "visibility", "hidden"

Also, == is converted to === so you'll want to compare against strings unless you want to "cast" your value to a number using selection = +select.val() (thanks to Trevor Burnham for this casting trick) or parseInt(select.val(), 10).

Upvotes: 26

Stan
Stan

Reputation: 4239

You can use switch:

switch selection
  when '2'
    price.html "$12"
    save.css "visibility", "visible"
  when '1'
    price.html "$5"
    save.css "visibility", "hidden"

Also you can take away return, because functions will always return their final value.

Upvotes: 6

Related Questions