patternrubyMinor
Reversing a number in Ruby
Viewed 0 times
numberreversingruby
Problem
I'm writing a function to reverse a number in Ruby. IE,
Here's what I have, which works:
I don't like the enumeration of different number types. Is there a way to dynamically pass in the type?
Something like:
Any other thoughts on simplifying this?
See also: My related Stack Overflow question on some other approaches I tried.
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
endI 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.
Solution
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,
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:
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
For multi-way conditionals like this, it is usually more legible to use a
Now we can start doing some real interesting changes.
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:
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
That is a big honking red flag:
What you are basically doing is re-implementing message dispatch:
will run different code, depending on the class of
So, we let Ruby take care of making the decision whether to invoke the
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:
Pull Up:
Now we're done. Our code is nice and clean, all our methods do exactly one thing, there are no
The only slightly smelly bit left is the monkey-patching we are doing. Refinements to the rescue!
[Well, ac
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
endConditionals 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
endBeing 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
endFor 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
endNow 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
endNow, 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' })
# FalseThis 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.barwill 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
endOkay. 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
endPull 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
endNow 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, ac
Code Snippets
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
endclass Numeric
def reverse
str = to_s.reverse
return if is_a?(Float)
str.to_f
elsif is_a?(Integer)
str.to_i
end
end
endclass Numeric
def reverse
str = to_s.reverse
if is_a?(Float)
str.to_f
elsif is_a?(Integer)
str.to_i
end
end
endclass Numeric
def reverse
str = to_s.reverse
case
when is_a?(Float)
str.to_f
when is_a?(Integer)
str.to_i
end
end
endclass Numeric
def reverse
str = to_s.reverse
case self
when Float
str.to_f
when Integer
str.to_i
end
end
endContext
StackExchange Code Review Q#94786, answer score: 6
Revisions (0)
No revisions yet.