HiveBrain v1.2.0
Get Started
← Back to all entries
patternrubyMinor

Spoj's 1st problem in tdd using Ruby

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
1stproblemspojusingrubytdd

Problem

I am pretty new to TDD and is following problems on spoj.pl for practice. I need help with following code; it was my first attempt.

Problem: Your program is to use the brute-force approach in order to find the Answer to Life, the Universe, and Everything. More precisely... rewrite small numbers from input to output. Stop processing input after reading in the number 42. All numbers at input are integers of one or two digits. Mentioned here.

Tests:

describe "life universe and everything" do

 before(:each) do 
  @life = Life.new
 end

 it "should accept input" do 
  @life.stub(:gets) { "5, 4, 23, 42,  4" }
  @life.input.should == [5,4,23,42,4]
 end

 it "check number validity" do
   @life.valid?(23).should be_true
   @life.valid?(42).should be_false
 end

 it "process input" do
   array1 = [5,4,23,42,4]
   @life.process_input(array1).should == [5,4,23]
   array2 = [1,4,76,34,90]
 @life.process_input(array2).should == [1,4,76,34,90]
end

end


Code:

class Life

 def input
  puts "List comma(',') seperated numbers. Press enter when done."
  numbers_string = gets
  numbers = numbers_string.split(',')
  numbers.map {|n| n.strip.to_i}
 end

 def valid?(number)
  (number==42) ? false : true
 end

 def process_input(numbers)
  processed_numbers = []
  numbers.each do |number|
  break unless valid?(number)
  processed_numbers << number
 end
  processed_numbers
end

end


I would like it to be reviewed and would be glad to know my mistakes and what can I do to improve quality of code. Thanks!

Solution

Congratulations. That's a pretty nice first attempt.

I see only a few issues:

-
The problem statement shows that you will receive one number per line; your code expects one line with multiple numbers, separated by commas.

-
The problem statement asks you to print the numbers; your program doesn't.

-
When run, the test writes its prompt to $stdout. Tests should be quiet when passing.

The issue with the prompt can be fixed with an expectation:

@life.should_receive(:puts)\
.with("List comma(',') separated numbers.  Press enter when done"


If your intent is to solve a slightly different problem, then we won't worry about the differences between what your program does and what they're asking for. But read below the fold for a simple test and implementation, TDD Style, for the stated problem.

This problem can be solved pretty simply. For example,

loop do
    n = gets.chomp.to_i
    puts n
    break if n == 42
  end


What test or tests could you write that would allow you to have code this simple? Let's see if we can get there from nothing:

class Life
  def process_input
  end
end

describe Life do

  it "should copy numbers from $stdin to $stdout" do
    life = Life.new
    life.should_receive(:gets).with(no_args).and_return('41')
    life.should_receive(:puts).with(41)
    life.process_input
  end

end


Running that, we find that the spec fails because the code never called gets. We add the call to gets, then find that the spec fails because the code never called puts. Adding them, we get a passing test:

class Life
  def process_input
    puts gets.to_i
  end
end


Of course, we've got no loop, so let's tell the test there should be multiple calls to gets and puts. We'll use .ordered to tell rspec that the calls should occur in a certain order.

it "should copy numbers from $stdin to $stdout" do
  life = Life.new
  life.should_receive(:gets).ordered.with(no_args).and_return('41')
  life.should_receive(:puts).ordered.with(41)
  life.should_receive(:gets).ordered.with(no_args).and_return('42')
  life.should_receive(:puts).ordered.with(42)
  life.process_input
end


Now there needs to be a loop. Let's add it:

def process_input
  loop do
    puts gets.to_i
  end
end


Our test neither passes nor fails. Instead, it hangs. That's because we didn't give it any way to get out of the loop. Let's add the termination condition:

def process_input
  loop do
    n = gets.to_i
    puts n
    break if n == 42
  end
end


So, we can write a test that lets us have pretty simple code. But in a few ways, this test is bothersome. One reason is that it is too picky about the way that the I/O is done. What if we changed the program to use print instead of puts? It would have produce exactly the same output, but the test would fail. Also, having the test hang when the program fails to terminate isn't very good. We want tests that pass or fail quickly and cleanly, with no guessing. So let's rewrite our test (and the program) using dependency injection for I/O.

Starting over, we're going to change the the Life program so that it takes and input object and an output object:

class Life

  def initialize(input = $stdin, output = $stdout)
    @input = input
    @output = output
  end

  def process_input
  end

end


And our test:

require 'stringio'

describe Life do

  it "should copy numbers from input to output" do
    input = StringIO.new("41\n")
    output = StringIO.new
    life = Life.new(input, output)
    life.process_input
    output.string.should == "41\n"
  end

end


Using StringIO instances as mock I/O objects is very handy, and lets us test the result of the I/O rather than the actions that produce the I/O. That's nice.

Since our program doesn't process anything yet, the test fails:

1) Life should copy numbers from $stdin to $stdout
     Failure/Error: output.string.should == "41\n"
       expected: "41\n"
            got: "" (using ==)


Let's fix that:

def process_input
    @output.print @input.gets
  end


The test now passes. Let's modify the test to show that it should loop:

it "should copy numbers from $stdin to $stdout" do
    input = StringIO.new("41\n42\n")
    output = StringIO.new
    life = Life.new(input, output)
    life.process_input
    output.string.should == "41\n42\n"
  end


The test fails:

1) Life should copy numbers from $stdin to $stdout
     Failure/Error: output.string.should == "41\n42\n"
       expected: "41\n42\n"
            got: "41\n" (using ==)


Let's add the loop:

def process_input
    loop do
      @output.print @input.gets
    end
  end


Now it hangs again! That's because I/O objects like StringIO and $stdin just return a nil when they run out of input. Our program is happily printing nil over and over. Let's make the test die nicely when the loop fails to terminate:

```
it "should copy numbers from $stdin to $stdout" do
input = St

Code Snippets

@life.should_receive(:puts)\
.with("List comma(',') separated numbers.  Press enter when done"
loop do
    n = gets.chomp.to_i
    puts n
    break if n == 42
  end
class Life
  def process_input
  end
end

describe Life do

  it "should copy numbers from $stdin to $stdout" do
    life = Life.new
    life.should_receive(:gets).with(no_args).and_return('41')
    life.should_receive(:puts).with(41)
    life.process_input
  end

end
class Life
  def process_input
    puts gets.to_i
  end
end
it "should copy numbers from $stdin to $stdout" do
  life = Life.new
  life.should_receive(:gets).ordered.with(no_args).and_return('41')
  life.should_receive(:puts).ordered.with(41)
  life.should_receive(:gets).ordered.with(no_args).and_return('42')
  life.should_receive(:puts).ordered.with(42)
  life.process_input
end

Context

StackExchange Code Review Q#15711, answer score: 6

Revisions (0)

No revisions yet.