Bubble sort implementation with tests

Question

For practice I tried implementing a bubble sort algorithm in Ruby. I'm especially interested in creating clean DRY and KISS code and keeping things readable, but efficient. Any hints to improve things would be very much appreciated. The code looks like this:

class Sorter
 def initialize
 end
 def sort(stack)
 if stack.length == 0
 return stack
 else
 checkArguments(stack)
 if stack.length == 1
 return stack
 else
 newstack = []
 stackIsSorted = false
 while !stackIsSorted
 newstack = iterateThroughStack(stack)
 if newstack == stack
 stackIsSorted = true
 else
 stack = newstack
 end
 end
 return newstack
 end
 end
 end
 def checkArguments(stack)
 stack.each do 
 |element| 
 if element.class != Float && element.class != Fixnum
 raise ArgumentError, "element detected in stack that is not an integer or double"
 end
 end
 end
 def iterateThroughStack(stack)
 newstack = []
 currentElement = stack[0]
 for i in (1...stack.length)
 if currentElement < stack[i]
 newstack.push(currentElement)
 currentElement = stack[i]
 else
 newstack.push(stack[i])
 end
 end
 newstack.push(currentElement)
 end
end

Then, after reading about Test Driven Development, I started using this practice and since then, I think code makes more sense with unit tests. So below are the unit test I wrote:

require 'test/unit'
require 'lib/Sorter'
class Test_Sorter < Test::Unit::TestCase
 def setup
 @sorter = Sorter.new
 end
 def test_emptyStack
 stack = []
 assert_equal(stack, @sorter.sort(stack), "sorting empty stack failed")
 end
 def test_StackWithOneElement
 stack = [1]
 assert_equal(stack, @sorter.sort(stack), "sorting stack with one element: 1 failed")
 stack = ["a"]
 assert_raise (ArgumentError) { @sorter.sort(stack) }
 end
 def test_StackWithTwoElements
 stack = [2, 1]
 sorted_stack = [1, 2]
 assert_equal(sorted_stack, @sorter.sort(stack), "sorting stack with two elements: 1, 2 failed")
 stack = [2, "a"]
 assert_raise (ArgumentError) { @sorter.sort(stack) }
 end
 def test_StackWithThreeElements
 stack = [2, 3, 1]
 sorted_stack = [1, 2, 3]
 assert_equal(sorted_stack, @sorter.sort(stack), "sorting stack with three elements: 1, 2, 3 failed")
 end
 def test_StackWithFourElements
 stack = [4, 2, 3, 1]
 sorted_stack = [1, 2, 3, 4]
 assert_equal(sorted_stack, @sorter.sort(stack), "sorting stack with four elements: 1, 2, 3, 4 failed")
 end
end

Answer 1

There are few things I would do differently:

1. Using class without instance variables doesn't make much sense, you can use module for that if the only thing you want is to limit scope. Of course it had been useful if you would have initialized class with some parameters like Sorter.new(Float, Fixnum) and saved them for later use,
2. When you iterate through the stack you know for sure if you swapped items or not, so I would say it's a good idea to save this information inside iterateThroughStack and pass it back so you don't need to compare arrays in sort,
3. Duck typing in Ruby means you normally don't check types, who cares what type of element is if it allows comparison, so I would remove checkArguments unless you have some special requirements,
4. I would use each instead of for,
5. Don't use return unless you want to return from the middle of a function (which is presumably bad thing to do).

So it all comes down to this:

class Sorter
 def initialize
 end
 def sort(stack)
 if stack.length > 1
 stackIsSorted = false
 while !stackIsSorted
 stackIsSorted, stack = iterateThroughStack(stack)
 end
 end
 stack
 end
 def iterateThroughStack(stack)
 stackIsSorted = true
 newstack = []
 currentElement, *tail = stack
 tail.each do |element|
 if currentElement < element
 newstack.push(currentElement)
 currentElement = element
 else
 newstack.push(element)
 stackIsSorted = false
 end
 end
 [stackIsSorted, newstack.push(currentElement)]
 end
end

• bartaelterman: Thanks Victor! I never heard of the *tail syntax, but found it here. Your code would require a test to be changed: sort(["a"]) will now return ["a"] in stead of raising an error. Isn't that an issue?
• Victor Moroz: It depends on your requirements, strings can be sorted too.

Answer 2

Just a note about the tests: I'd create a test method for every assert call. For example,

 def test_StackWithTwoElements
 stack = [2, 1]
 sorted_stack = [1, 2]
 assert_equal(sorted_stack, @sorter.sort(stack), "sorting stack with two elements: 1, 2 failed")
 stack = [2, "a"]
 assert_raise (ArgumentError) { @sorter.sort(stack) }
 end

would be

 def test_StackWithTwoElements
 stack = [2, 1]
 sorted_stack = [1, 2]
 assert_equal(sorted_stack, @sorter.sort(stack), "sorting stack with two elements: 1, 2 failed")
 end
 def test_StackWithInvalidElement
 stack = [2, "a"]
 assert_raise (ArgumentError) { @sorter.sort(stack) }
 end

Too many asserts in one test is a bad smell. It's Assertion Roulette and you lost Defect Localization. If the first assert_equal throws an exception you don't know anything about the results of the other assert calls which could be important because they could help debugging and defect localization.

• bartaelterman: That's a nice suggestion. Thanks for sharing the links too. Interesting stuff.