Search code examples
rubyif-statementfixnum

Why is this Ruby if/else loop running more than once, and only partially on the second time?


I have a method I'm creating to take a Fixnum input and to return the number spelt out in words. The method works for the most part, but with certain inputs, I'm getting some whacky outputs.

And when I pry into the code, it seems like this line of code:

# .concat(' hundred ').concat(reference.fetch(thousands%100)).concat(' thousand ')

which is only a part of an if/else statement and is running itself twice.

111230.numbers_to_words() correctly returns: "one hundred eleven thousand two hundred thirty"

but

111130.numbers_to_words() incorrectly returns: "one hundred eleven thousand one hundred eleven thousand hundred thirty"

"hundred eleven thousand " being the undesired string element that's returned from an if/else statement that's already run itself successfully.

What is going on? I thought if/else statements once run can't be processed again. What is causing that mysterious string to be returned?

class Fixnum
  define_method(:numbers_to_words) do
    remaining = self
    output = ""
    reference = { 1 => 'one', 2 => 'two', 3 => 'three', 4 => 'four', 5 => 'five', 6 => 'six', 7 => 'seven', 8 => 'eight', 9 => 'nine',
                  10 => 'ten', 11 => 'eleven', 12 => 'twelve', 13 => 'thirteen', 14 => 'fourteen', 15 => 'fifteen', 16 => 'sixteen', 17 => 'seventeen',
                  18 =>  'eighteen', 19 => 'nineteen',
                  20 => 'twenty', 21 => 'twenty one', 22 => 'twenty two', 23 => 'twenty three', 24 => 'twenty four', 25 => 'twenty five', 26 => 'twenty six',
                  27 => 'twenty seven', 28 => 'twenty eight', 29 => 'twenty nine',
                  30 => 'thirty', 31 => 'thirty one', 32 => 'thirty two',
                  33 => 'thirty three', 34 => 'thirty four', 35 => 'thirty five', 36 => 'thirty six', 37 => 'thirty seven', 38 => 'thirty eight', 39 => 'thirty nine',
                  40 => 'forty', 41 => 'forty one', 42 => 'forty two', 43 => 'forty three', 44 => 'forty four', 45 => 'forty five', 46 => 'forty six', 47 => 'forty seven',
                  48 => 'forty eight', 49 => 'forty nine', 50 => 'fifty', 51 => 'fifty one', 52 => 'fifty two', 53 => 'fifty three', 54 => 'fifty four', 55 => 'fifty five',
                  56 => 'fifty six', 57 => 'fifty seven', 58 => 'fifty eight', 59 => 'fifty nine', 60 => 'sixty', 61 => 'sixty one', 62 => 'sixty two', 63 => 'sixty three',
                  64 => 'sixty four', 65 => 'sixty five', 66 => 'sixty six', 67 => 'sixty seven', 68 => 'sixty eight', 69 => 'sixty nine', 70 => 'seventy', 71 => 'seventy one',
                  72 => 'seventy two', 73 => 'seventy three', 74 => 'seventy four', 75 => 'seventy five', 76 => 'seventy six', 77 => 'seventy seven', 78 => 'seventy eight',
                  79 => 'seventy nine', 80 => 'eighty', 81 => 'eighty one', 82 => 'eighty two', 83 => 'eighty three', 84 => 'eighty four', 85 => 'eighty five',
                  86 => 'eighty six', 87 => 'eighty seven', 88 => 'eighty eight', 89 => 'eighty nine', 90 => 'ninety', 91 => 'ninety one', 92 => 'ninety two',
                  93 => 'ninety three', 94 => 'ninety four', 95 => 'ninety five', 96 => 'ninety six', 97 => 'ninety seven', 98 => 'ninety eight', 99 => 'ninety nine' }

    if remaining > 0 && remaining < 1000000000000
      if remaining >= 1000000000 && remaining < 1000000000000
        billions = remaining/1000000000
        remaining = remaining%1000000000

        if billions.to_s().length() > 2 && billions >= 100 && billions%100 != 0
          output = output + reference.fetch(billions/100).concat(' hundred ').concat(reference.fetch(billions%100)).concat(' billion ')
        elsif billions.to_s().length() > 2 && billions >= 100
          output = output.concat(reference.fetch(billions/100)).concat(' hundred ').concat(' billion ')
        elsif billions.to_s().length() < 3 && billions > 0 && billions < 100
          output = output.concat(reference.fetch(billions)).concat(' billion ')
        else
          output = output + " "
        end
      end

      if remaining >= 1000000 && remaining < 1000000000
        millions = remaining/1000000
        remaining = remaining%1000000

        if millions.to_s().length() > 2 && millions >= 100 && millions%100 != 0
          output = output + reference.fetch(millions/100).concat(' hundred ').concat(reference.fetch(millions%100)).concat(' million ')
        elsif millions.to_s().length() > 2 && millions >= 100
          output = output.concat(reference.fetch(millions/100)).concat(' hundred ').concat(' million ')
        elsif millions.to_s().length() < 3 && millions > 0 && millions < 100
          output = output.concat(reference.fetch(millions)).concat(' million ')
        else
          output = output + " "
        end
      end

      if remaining >= 1000 && remaining < 1000000 #111110
        thousands = remaining/1000 #111 length is 3
        remaining = remaining%1000 #110

        if thousands.to_s().length() > 2 && thousands >= 100 && thousands%100 == 0
        output = output + reference.fetch(thousands/100).concat(' hundred ').concat(' thousand ')
        elsif thousands.to_s().length() > 2 && thousands >= 100 && thousands%100 != 0
          output = output + reference.fetch(thousands/100).concat(' hundred ').concat(reference.fetch(thousands%100)).concat(' thousand ')
        elsif thousands.to_s().length() < 3 && thousands > 0 && thousands < 100
          output = output.concat(reference.fetch(thousands)).concat(' thousand ')
        else
          output = output + " "
        end
      end

      # .concat(' hundred ').concat(reference.fetch(thousands%100)).concat(' thousand ')

      if remaining >=100 && remaining < 1000

        if remaining>100 && remaining<1000 && remaining%100 > 0
          output = output + reference.fetch(remaining/100).concat(' hundred ').concat(reference.fetch(remaining%100))
        elsif remaining>100 && remaining<1000 && remaining%100 == 0
          output = output + reference.fetch(remaining/100).concat(' hundred ')
        elsif remaining == 100
          output = output + ('one hundred')
        else
          output = output + " "
        end

      end

      if remaining < 100 && remaining > 0
        output = output.concat(reference.fetch(remaining))
      end

    else
      output = "zero"
    end
    output.strip().squeeze(" ")
  end
end

Solution

  • The problem is all the lines of the form

    reference.fetch(something).concat(" something else")
    

    concat modifies the receiver (as opposed to returning a new string) so all of these lines are changing the values in your reference hash. In some cases you don't use that any of the values in the hash more than once and so you don't notice the problem, in other cases you do.

    Replacing that usage with

    reference.fetch(something) + "something else"
    

    Would avoid this

    You might consider freezing the strings in your hash - this would result in this sort of error raising an exception rather than silently misbehaving.