Reversing a number in Ruby

Question

I'm writing a function to reverse a number in Ruby. IE, 314159.reverse should return 951413.

Here's what I have, which works:

class Numeric
  def reverse
    str = self.to_s.reverse

    if self.is_a?(Float)
      return str.to_f
    elsif self.is_a?(Integer)
      return str.to_i
    end
  end
end

I don't like the enumeration of different number types. Is there a way to dynamically pass in the type?

Something like:

return str.to_type(self.type)

Any other thoughts on simplifying this?

See also: My related Stack Overflow question on some other approaches I tried.

Answer 1

I am going to refactor this in small steps so that you can follow my thought process and see where you agree and you disagree.

First, let's start with some style. In Ruby, self is the implied receiver of a message send if you don't supply one, it is generally not necessary to state it explicitly:

class Numeric
  def reverse
    str = to_s.reverse

    if is_a?(Float)
      return str.to_f
    elsif is_a?(Integer)
      return str.to_i
    end
  end
end

Conditionals are expressions in Ruby, not statements. In fact, everything is an expression in Ruby. So, conditionals evaluate to a value, which means that instead of returning a value from each of the branches, we can return the value of the entire conditional expression instead:

class Numeric
  def reverse
    str = to_s.reverse

    return if is_a?(Float)
      str.to_f
    elsif is_a?(Integer)
      str.to_i
    end
  end
end

Being an expression-oriented language, the value of a method body (and also a block body, module body, class body, any block of code, really) is the value of the last expression evaluated inside that body. There is no need to explicitly return (or next in case of a block) it:

class Numeric
  def reverse
    str = to_s.reverse

    if is_a?(Float)
      str.to_f
    elsif is_a?(Integer)
      str.to_i
    end
  end
end

For multi-way conditionals like this, it is usually more legible to use a case expression, instead of a series of if and elsifs:

class Numeric
  def reverse
    str = to_s.reverse

    case
    when is_a?(Float)
      str.to_f
    when is_a?(Integer)
      str.to_i
    end
  end
end

Now we can start doing some real interesting changes. Module#=== checks whether its argument is an instance of self. Object#is_a? checks whether self is an instance of its argument. IOW: the two methods are mirror images of each other, you can usually exchange o.is_a?(m) with m === o and vice-versa. This allows us to simplify our case expression even more:

class Numeric
  def reverse
    str = to_s.reverse

    case self
    when Float
      str.to_f
    when Integer
      str.to_i
    end
  end
end

Now, on to the big thing!

In an object-oriented language, it is always possible to replace a conditional with runtime polymorphic message dispatch. Message dispatch is more powerful than conditionals. Smalltalk is the existence proof of that, it doesn't even have conditionals built into the language, its conditionals are implemented in the library, using message dispatch, kind of like this:

class TrueClass
  def if_then_else(then_block, else_block)
    then_block.()
  end
end

class FalseClass
  def if_then_else(then_block, else_block)
    else_block.()
  end
end

(2 > 3).if_then_else(-> { puts 'True' }, -> { puts 'False' })
# False

This gives rise to the Replace Conditional with Polymorphism Refactoring. (Here is an example of it in Ruby.)

In your case, you are not just switching behavior based on some abstract notion of "type" of the object, you are literally switching behavior based on the class. Even worse: you are switching based on the class of self.

That is a big honking red flag: self always knows what class it is. It should never ever have to check for its own class!

What you are basically doing is re-implementing message dispatch:

foo.bar

will run different code, depending on the class of foo. That's already built into Ruby, you don't have to re-implement it yourself.

So, we let Ruby take care of making the decision whether to invoke the Float or the Integer version of our code:

class Integer
  def reverse
    str = to_s.reverse

    str.to_i
  end
end

class Float
  def reverse
    str = to_s.reverse

    str.to_f
  end
end

Okay. So, the last bit we have here is a little bit of code duplication, which we can get rid of by using the Extract Method Refactoring followed by the Pull Up Method Refactoring.

Extract:

class Integer
  def reverse
    reverse_to_s.to_i
  end

  private def reverse_to_s
    to_s.reverse
  end
end

class Float
  def reverse
    reverse_to_s.to_f
  end

  private def reverse_to_s
    to_s.reverse
  end
end

Pull Up:

class Numeric
  private def reverse_to_s
    to_s.reverse
  end
end

class Integer
  def reverse
    reverse_to_s.to_i
  end
end

class Float
  def reverse
    reverse_to_s.to_f
  end
end

Now we're done. Our code is nice and clean, all our methods do exactly one thing, there are no IFs or other conditionals.

The only slightly smelly bit left is the monkey-patching we are doing. Refinements to the rescue!

module ReversibleNumeric
  refine Numeric do
    private def reverse_to_s
      to_s.reverse
    end
  end

  refine Integer do
    def reverse
      reverse_to_s.to_i
    end
  end

  refine Float do
    def reverse
      reverse_to_s.to_f
    end
  end
end

314159.reverse
# NoMethodError

using ReversibleNumeric

314159.reverse
# => 951413

[Well, actually, Refinements are still in a state of flux, and the above code may or may not work, depending on your exact version and implementation of Ruby.]

Answer 2

How about using a case statement.

def reverse(n)
  as_string = n.to_s.reverse
  case n
  when Float
    Float(as_string)
  when Integer
    Integer(as_string)
  end
end

reverse(1234.5678) # => 8765.4321 A Float
reverse(12343) # => 34321 which is an Integer
reverse(832987979497908) # => 809794979789238 A Bignum
reverse(0371) # => 942 An Octal
reverse(0x1f) # => 13 A Hexadecimal

Answer 3

Modifying the internals of a class is usually frowned upon, also, I feel like reversing a float is not going to be useful, so you can greatly simplify when caring only about integers:

def number_reverse(n)
  n.to_s.reverse.to_i
end

p number_reverse(12345) #-> 12345