Ruby bubble sort algorithm

Question

I'm just looking for constructive criticism of my Ruby implementation of a bubble sort algorithm.

class BubbleSorter < Object
 def bubble_sort(list)
 swaps = 0
 # parse the list until it is sorted
 until @sorted == true
 # run the comparison of adjacent elements
 for i in 0...(list.length - 1)
 # if the first is greater than the second, swap them with parallel assignment
 if list[i] > list[i+1]
 list[i], list[i+1] = list[i+1], list[i]
 # increase the number of swaps performed in this run by 1
 swaps += 1
 end
 # compare the next 2 elements
 i += 1
 end
 # uncomment the following line to see each iteration:
 # p list
 # If any swaps took place during the last run, the list is not yet sorted
 if swaps > 0
 @sorted = false
 # no swaps? Everything is in order
 else
 @sorted = true
 end
 # reset swap count for each run
 swaps = 0
 end
 end
end

Answer 1

Although you should comment your work, some of your comments seem unecessary. Let's look at some lines to see why:

# parse the list until it is sorted
until @sorted == true

The comment seems redundant. Your line of code until @sorted == true reads clear enough for me to understand that you want to keep doing something until the list is sorted.

# if the first is greater than the second, swap them with parallel assignment
if list[i] > list[i+1]
 list[i], list[i+1] = list[i+1], list[i]

This code is also pretty self explanatory. If you did have a comment you should be explaining why you do this. However, bubblesort is pretty well known so you probably don't even need a comment on this.

# increase the number of swaps performed in this run by 1
swaps += 1

Again why? I can read the code, it is obvious you're incrementing the number of swaps. I would omit.

# compare the next 2 elements
i += 1

Based of the if statement I would know that this the purpose of i. I would omit it, but it does explain why you increment i, so there is some grey zone here.

You write some good comments toward the end, let's analyze why they are good:

# If any swaps took place during the last run, the list is not yet sorted
if swaps > 0
 @sorted = false
# no swaps? Everything is in order
else
 @sorted = true

It explains why we need to check if swaps > 0 and how the sorting algorithm depends on this.

Answer 2

No need for @sorted variable

You can remove the @sorted variable and return directly if no swaps were performed, change the loop to while true and add return if swaps == 0 instead of the if swaps ... @sorted =... statements.

No need for a class

You also do not need a class for this, you can either write a top-level function or put it into a module, a class is used to hold data and functions to operate on it, a class that holds a single static function is unreasonable.

Ruby step method

You should use the .step(2) method as it is more explicit than manual incrementing and modyfing a loop variable inside the body is unusual and breaks the expectation that we already know how many times a for loop should run.