I just finished writing a basic binary conversion class in Ruby. I'm curious to see if anyone has suggestions for improvements. In particular, I'm looking for the cleanest, shortest, most succinct code possible, while staying away from hacky shortcut stuff.

In creating this I wanted to use a minimum of functions that come with Ruby by default, as that would sort of defeat the purpose of this as a practice program. For instance, the entire thing could have been summed up with a quick line like this.

"101".to_i(2) ##> 5

But I wanted to do most of the math by hand, so to speak.

There is one place in the get_bits method I did use a string, and I'm not too happy about it. However it was the only way I could think of to get the bits into an array. If there's another way to do so without splitting a string, I'd be glad to hear it.

Here's the class.

class Converter
  def initialize binary_number
    @binary = binary_number
    @bits = get_bits
    @total = 0

  def to_base_ten
    @bits.reverse.each_with_index do |bit, power|
      append_to_total bit_value(bit, power)

  def get_bits
    @binary.to_s.split('').map do |bit|

  def validate_binary
    @bits.each do |bit|
      raise "Invalid binary!" unless bit == 0 || bit == 1

  def bit_value bit, power
    is_set?(bit) ? two_to_power_of(power) : 0

  def two_to_power_of power
    total = 1
    power.times do
      total = total * 2

  def is_set? bit
    bit == 1

  def append_to_total amount
    @total = @total + amount

  def get_total

I also used RSpec to test it as best I could, but I won't post the specs here, just because they're long and not directly related to the class itself. If you want to inspect those too, they can be found on GitHub.

So there ya have it. Any thoughts are greatly appreciated. Clean code forever!


1 Answer 1

  1. The name Converter is pretty ambiguous. It could mean anything.

  2. I'd suggest doing less work in the initializer, and more lazy evaluation.

  3. Skip that self line in the initializer - it's an initializer; it'll always return self (or, technically, it doesn't matter what the initializer returns, since you'll be calling new and that will return the new instance. Point is, skip that line)

  4. Use Enumerable#inject to do the sum

  5. Use String#chars instead of String#split

  6. It'd probably be better to raise right away in the initializer if the string isn't a binary number. Also better to raise an ArgumentError while you're at it.

  7. You can check the string with a simple regular expression: /^[01]*$/

  8. Your #two_to_power_of method can be replaced with 2 ** n

  9. Don't use prefixes like is_ and get_ in your method names (specifically, is_set?, get_bits and get_total); it's not Ruby's style. For instance, the nil? method isn't called is_nil?, and the chars method mentioned above isn't called get_chars.
    All methods return something so a get_ is redundant, and a ? suffix is a substitute for an is_ prefix, just like a = suffix is a substitute for a set_ prefix.

  10. Favor accessor methods over directly accessing instance variables. Such indirections allow for better internal decoupling. That is, as soon as you start accessing a value through a method, you make your class more flexible. You can begin with simple attr_reader-synthesized methods, which will of course not do anything special, but should you ever need or want to add more logic, you need only change the accessor method(s). In this case, however, a first step would be to simply use fewer instance variables (see #2)

  11. A class method might be nice for quick conversions, so you don't always have to deal with instantiating an object. E.g. a class method could allow you to say Converter.convert("101010") and get 42 directly. It'd just be a shortcut for Converter.new("101010").to_base_ten, but that's still a nice shortcut to have.

  12. Might be nice if you could specify endianness of the binary number, instead of assuming it's always little endian.

In the end, I get something like this, if we want memoization of the converted values:

class BinaryNumberConverter
  attr_reader :binary

  def self.convert(string, little_endian = true)

  def initialize(binary)
    if binary =~ /^[01]*$/
      @binary = binary
      raise ArgumentError.new("'#{binary}' is not a valid binary number")

  def base_ten(little_endian = true)
    little_endian ? little_endian_value : big_endian_value


  def little_endian_value
    @little_endian ||= convert(binary.reverse)

  def big_endian_value
    @big_endian ||= convert(binary)

  def convert(string)
    string.chars.each_with_index.inject(0) do |sum, digit|
      char, index = digit
      sum += char == "1" ? 2 ** index : 0


instance = BinaryNumberConverter.new("101010")
instance.base_ten        # => 42
instance.base_ten(false) # => 21

BinaryNumberConverter.convert("101010")        # => 42
BinaryNumberConverter.convert("101010", false) # => 21

Of course, it'd be a lot simpler to just extend String rather than make an entire class:

class String
  def bin_to_dec(little_endian = true)
    raise "'#{self}' is not a valid binary number" unless self =~ /^[01]*$/
    digits = little_endian ? reverse.chars : chars
    digits.each_with_index.inject(0) do |sum, digit|
      char, index = digit
      sum += char == "1" ? 2 ** index : 0

And you get

"101010".bin_to_dec        #=> 42
"101010".bin_to_dec(false) #=> 21

No memoization, but I'd call that pretty clean.

Took a quick look at your tests, and it's a bit overkill (also, you should use the expect syntax of rspec; the should syntax is old-school).

I'd actually just do something like

number = rand(12345) # can be anything really
string = number.to_s(2)
expect(string.bin_to_dec).to eq(number)

And call it good. You'll note I'm using to_s(2) above, but here it makes sense. We have to assume that Ruby works anyway, so in this case, it's a perfect yardstick. Similarly, to check big endian conversion, we can use to_i(2) with a clear conscience

string = rand(12345).to_s(2).reverse
expect(string.bin_to_dec(false)).to eq(string.to_i(2))

Add a test for the exception raising, and you've tested everything.

On another note, you say you "wanted to use a minimum of functions that come with Ruby by default as that would sort of defeat the purpose of this as a practice program". But I'd recommend you use as much built-in functionality as possible, especially in your practice programs (in this case stopping short of using String#to_i, of course).

Learning any language is usually less about learning the language itself (as in syntax), as it is about learning the conventions, idioms, and the built-in goodies. Intentionally doing things "the hard way" will probably teach you the wrong lessons. And the code you write won't teach you conventions and idioms, because it's unconventional to make things hard for yourself and no idioms exist for it.

Besides, at first, the easy solution will actually be the difficult one, because the easy solution is the one that requires experience. But brute-forcing your way though a problem won't earn you that experience and will actually teach you a lot less than trying to use the language to your advantage.

  • \$\begingroup\$ Can you please expand on points 9 and 10? All the others get a resounding +1 from me. \$\endgroup\$ Apr 19, 2014 at 2:40
  • 1
    \$\begingroup\$ @DavidHarkness Thanks! I've fleshed it out a bit more; let me know if it's still lacking \$\endgroup\$
    – Flambino
    Apr 19, 2014 at 3:04
  • \$\begingroup\$ Nope, that was perfect. \$\endgroup\$ Apr 19, 2014 at 3:13
  • \$\begingroup\$ Thanks for the excellent answer. There's a lot of useful information there. \$\endgroup\$
    – Scotty C.
    Apr 20, 2014 at 2:18

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.