How to exclude an unused case by conditions

Question

Which is better?

string.each_char do |letter|
    unless letter == " "
        if letters.has_key?(letter)
            letters[letter] = letters[letter].next
        else
            letters[letter] = 1
        end
    end
end

or

string.each_char do |letter|
    if letter == " "
    elsif letters.has_key?(letter)
        letters[letter] = letters[letter].next
    else
        letters[letter] = 1
    end
end

It seems awkward to leave an if statement without the body, but it also seems preferable to going a step deeper with unless.

There's a better way to write this code. I didn't know about default Hash values, and that would clean this code up a lot, but I still wanted to know which is preferable: an if statement without body, or unless, or something else.

Answer 1

This is probably the nicest:

letters = Hash.new(0)
string = "aaabbc"
string.each_char do |letter|
    if letter != " "
        letters[letter] += 1
    end
end
# => {"a"=>3, "b"=>2, "c"=>1}

For deciding between your two examples, I would avoid adding extra-depth (more indentation). The second one is also easier to read because it's simple to follow a string of if/else statements. It's almost always preferable to have more readable code than fancy code.