Coffeescript if ignoring line of code

Question

I am not particularly experienced with javascript or coffeescript but I have managed to create a simple function using coffeescript and jQuery. The function is used to highlight and show/hide games for a tournament organising website so that it automatically hides and unchecks the games that shouldn't be selected and only shows one undecided game at a time. The function works perfectly with all but one caveat:

validate = ->
finished = true
$('.edit').find('.round').find('.match').each (m, match) ->
    wins = {}
    valid = true
    $(match).find('.game').each (g, game) ->
        if valid
            $(game).show()
            $(game).find('label').css('background-color', 'red')

            checked = $(game).find(':checked')

            if checked.length == 0
                valid = false

            else
                winner = checked.attr('value')
                if winner of wins
                    if ++wins[winner] == 2
                        valid = false
                else
                    wins[winner] = 1
                $('label[for=' + checked.attr('id') + ']').css('background-color', 'green')


        else
            $(game).find(':checked').prop('checked', false)
            $(game).hide()

    if finished
        done = false
        for p, w of wins
            if w == 2
                done = true
        if not done
            finished = false

    if finished
        $('input:submit').show()
    else
        $('input:submit').hide()

The function will refuse to work unless I write something, anything, after

if checked.length == 0
    valid = false

on the same indentation as the valid = false

And I mean anything. I could set a new variable there and it works fine!

I have scoured around trying to find out if this is some indentation error and it just ignored the line. I've tried

valid = false if checked.length == 0

But I just get an error from rails because I have an else statement after that if. Please can someone explain to me what is going on.

What exactly does "refuse to work" mean? And what is the exact error message you get if you use `valid = false if checked.length == 0`? — mu is too short, Sep 24 '14 at 17:36
Two things. 1. What does "refuse to work" mean? Do you get error messages in your console? What happens instead of what you expected? 2. Are you sure you aren't mixing tabs and spaces? You have to watch out for that. Most coffee script is written with 2 space indentation, so I'd recommend replacing all tabs with two spaces. — Alex Wayne, Sep 24 '14 at 17:36
@AlexWayne: That's sort of irrelevant, any wall of code should include the exact error messages (including line numbers). — mu is too short, Sep 24 '14 at 17:50

score 0 · Answer 1 · answered Sep 24 '14 at 17:38

There is no reason for having an if sentence only to use the else part.

Instead you should use unless. So instead of this:

if checked.length == 0
  valid = false
else
  winner = checked.attr('value')
  ..

You could do this:

unless checked.length == 0
  winner = checked.attr('value')
  ..

or just:

if checked.length
  winner = checked.attr('value')
  ..

score 0 · Accepted Answer · edited May 23 '17 at 11:54

I apologise to everyone for not explaining my code in greater detail.

My problem was that if I didn't add another line after the value = false statement my function would not hide or recolour the rest of the games. I am sorry that I did not mention this.

The if clause in question was verifying whether there were any selected radio buttons which is how I was collecting the information which I also did not specify and am sorry. If no radio button was checked then the function would set valid to false so that all future games would be hidden and unchecked as well. If there were checked items then the function calculated who won and recoloured them accordingly, invalidating all future games in the match if someone won twice.

After having a closer look at my converted javascript using js2coffee.org I found that coffee script was inserting implicit returns at certain lines including the value = false, since the preprocessor determines that this is the last line of code run should it be reached by the logic and according to this gentleman here if you return false in an each block it will break the loop.

In order to fix this I needed to add an explicit return at the end of each block to signify that I wish to return nothing:

$('.edit').find('.round').find('.match').each (m, match) ->
    wins = {}
    valid = true
    $(match).find('.game').each (g, game) ->
        if valid
            $(game).show()
            $(game).find('label').css('background-color', 'red')

            checked = $(game).find(':checked')

            if checked.length is 0
                valid = false
            else
                winner = checked.attr('value')
                if winner of wins
                    valid = false if ++wins[winner] is 2
                else
                    wins[winner] = 1
                $('label[for=' + checked.attr('id') + ']').css('background-color', 'green')
        else
            $(game).find(':checked').prop('checked', false)
            $(game).hide()
        return
    return

I also changed == to is and some of the if clauses to be one liners for conciseness.

Coffeescript if ignoring line of code

2 Answers2