Binary converter and sufficient tests

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
    self
  end

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

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

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

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

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

  def is_set? bit
    bit == 1
  end

  def append_to_total amount
    @total = @total + amount
  end

  def get_total
    @total
  end
end

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 fore

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

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

-
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)

-
Use Enumerable#inject to do the sum

-
Use String#chars instead of String#split

-
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.

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

-
Your #two_to_power_of method can be replaced with 2 ** n

-
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.

-
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)

-
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.

-
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)
    self.new(string).base_ten(little_endian)
  end

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

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

  private

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

  def big_endian_value
    @big_endian ||= convert(binary)
  end

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

Usage:

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
    end
  end
end

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 (

StackExchange Code Review Q#47622, answer score: 7