# Ruby Kata Gem: Calculator

I've just completed the Calculator Kata in the Kata Gem and I was hoping for some feedback. Here's the Kata:

Calculator Kata
Create a calculator that is initialized with a string expression
- detail: The expression is of the form digits separated by commas: "1,2"
- detail: The expression is accessed by a method named expr
- detail: The expression can be reset for evaluation at any time without re-initializing
- example: Calculator.new "1,2"

completed (Y|n): y

Create an add method that sums the string expression
- detail: The method will return the sum of the digits
- detail: The expression can contain 0, 1 or 2 numbers
- detail: Then empty string will return 0
- example: "" computes to 0
- example: "1" computes to 1
- example: "1,2" computes to 3

completed (Y|n):

Allow the expression to contain an unknown amount of numbers
- example: "1,2,3" computes to 6
- example: "1,2,5,8" computes to 16

completed (Y|n):

Diff Method
Create a diff method that computes the consecutive differences
- detail: The expression must contain at least 2 digits
- example: "1,0" compues to 1
- example: "3,2,1" computes to 0
- example: "5,4,3,2,1" computes to -5
- detail: Expressions with less than 2 digits raise an exception
- example: "" or "5"

completed (Y|n):

Prod Method
Create a prod method that computes the multiples in the expression
- detail: The method will return the product of the numbers
- example: "0" computes to 0
- example: "2,1" computes to 2
- example: "3,2,1" computes to 6

completed (Y|n):

Div Method
Create a div method that computes the consecutive divisions in the expression
- detail: The method will return the final quotient of the numbers
- detail: it will raise an exception if the expression contains the number 0
- example: "2,1" computes to 2
- example: "3,2,1" computes to 1
- example: "1,2,3" computes to 0

completed (Y|n):

Congratulations!
- Create a calculator that is initialized with a string expression         01:15:02
- Create an add method that sums the string expression                     14:34:41
- Allow the expression to contain an unknown amount of numbers             00:04:33
- Create a diff method that computes the consecutive differences           00:06:14
- Create a prod method that computes the multiples in the expression       00:20:07
- Create a div method that computes the consecutive divisions in the exp   00:11:54
----------------------------------------------------------------------     --------
Total Time taking Calculator kata:                                         16:32:31


And, here is my solution (specs and the Calculator class are in the same file):

class Calculator
def initialize(expression="")
@expression = expression.split(",").map(&:to_i) || 0
if !@expression.any?
@expression = 
end
end

def expr
@expression
end

def sum
@expression.inject(&:+) || 0
end

def diff
if @expression.compact.count < 2
raise "expect 2 or more elements but recieved #{@expression.compact.count}"
end
@expression.inject(&:-)
end

def prod
@expression.inject(&:*)
end

def div
if @expression.find { |element| element == 0 }
raise "divide by zero"
end
@expression.inject(&:/)
end
end

describe "Calculator" do
subject(:calculator) { Calculator.new(expression) }

shared_examples "methods" do
specify { expect { calculator }.to_not raise_exception }

it { should respond_to(:expr) }
it { should respond_to(:sum)  }
it { should respond_to(:diff) }
it { should respond_to(:prod) }
it { should respond_to(:div)  }
end

shared_examples "too few elements" do
specify { expect { calculator.diff }.to raise_exception }
end

shared_examples "divide by zero" do
specify { expect { calculator.div }.to raise_exception }
end

context "with argument ''" do
let(:expression) { "" }

it_behaves_like "methods"
it_behaves_like "too few elements"
it_behaves_like "divide by zero"

its(:expr) { should eq() }
its(:sum)  { should eq(0)   }
its(:prod) { should eq(0)   }
end

context "with argument '1'" do
let(:expression) { "1" }

it_behaves_like "methods"
it_behaves_like "too few elements"

its(:expr) { should eq() }
its(:sum)  { should eq(1)   }
its(:prod) { should eq(1)   }
end

context "with argument '1,2'" do
let(:expression) { "1,2" }

it_behaves_like "methods"

its(:expr) { should eq([1,2]) }
its(:sum)  { should eq(3)     }
its(:diff) { should eq(-1)    }
its(:prod) { should eq(2)     }
end

context "with argument '1,2,3," do
let(:expression) { "1,2,3" }

it_behaves_like "methods"

its(:expr) { should eq([1,2,3]) }
its(:sum)  { should eq(6)       }
its(:diff) { should eq(-4)      }
its(:prod) { should eq(6)       }
its(:div)  { should eq(0)       }
end

context "with argument '1,2,5,8'" do
let(:expression) { "1,2,5,8" }

it_behaves_like "methods"

its(:expr) { should eq([1,2,5,8]) }
its(:sum)  { should eq(16)        }
its(:diff) { should eq(-14)       }
its(:prod) { should eq(80)        }
end

context "with argument '1,0'" do
let(:expression) { "1,0" }

it_behaves_like "divide by zero"

its(:diff) { should eq(1) }
its(:prod) { should eq(0) }
end

context "with argument '3,2,1'" do
let(:expression) { "3,2,1" }

its(:diff) { should eq(0)   }
its(:prod) { should eq(6)   }
its(:div)  { should eq(1) }
end

context "with argument '5,4,3,2,1,'" do
let(:expression) { "5,4,3,2,1" }

its(:diff) { should eq(-5)  }
its(:prod) { should eq(120) }
end

context "with arguments '2,1'" do
let(:expression) { "2,1" }

its(:div) { should eq(2) }
end
end


Looks good! Most importantly, you're using inject properly. The specs seem nice too (although running all the "methods" expectations every time is maybe a little redundant).

But I don't see support for this requirement:

The expression can be reset for evaluation at any time without re-initializing

Right now, you only have a reader method for expr.
Also, that reader returns an array - not the original string. While it's not stated directly, I think it's implied that you should get a string back. calculator.expr = calculator.expr should be idempotent.

There's also something weird going on in your current initialize method:

@expression = expression.split(",").map(&:to_i) || 0


I don't see how it would ever get to the OR (split will either give you an empty array or raise an exception), but even if it did, it'd just raise an exception in the next line:

if !@expression.any?


since Fixnum doesn't respond to any?.

But it wouldn't take much to fix all of this. Something like this should be OK

class Calculator

def expr=(string='')
@expr = string
@expression = @expr.split(",").map(&:to_i)
@expression =  if @expression.none?
end

alias_method :initialize, :expr=

#...
end


But I'd probably rename @expression to @values or something, just to make the separation clearer (and to give the array a pluralized name).

The div method can be made simpler, as this

 if @expression.find { |element| element == 0 }


can be expressed as just:

 if @expression.include?(0)


And lastly, you may want to raise the built-in ZeroDivisionError instead of a custom one:

 raise ZeroDivisionError if @expression.include?(0)

• Thank you so much for your feedback. This is really very helpful. – user341493 Nov 5 '13 at 17:16
• @user341493 No prob. Don't forget to click the checkmark to mark your question as answered (no rush though) – Flambino Nov 5 '13 at 17:34