snippetrubyMinor
Ruby bubble sort algorithm
Viewed 0 times
sortbubblealgorithmruby
Problem
I'm just looking for constructive criticism of my Ruby implementation of a bubble sort algorithm.
class BubbleSorter 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
endSolution
Although you should comment your work, some of your comments seem unecessary. Let's look at some lines to see why:
The comment seems redundant. Your line of code
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.
Again why? I can read the code, it is obvious you're incrementing the number of
Based of the
You write some good comments toward the end, let's analyze why they are good:
It explains why we need to check if
# parse the list until it is sorted
until @sorted == trueThe 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 += 1Again 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 += 1Based 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 = trueIt explains why we need to check if
swaps > 0 and how the sorting algorithm depends on this.Code Snippets
# parse the list until it is sorted
until @sorted == true# 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# compare the next 2 elements
i += 1# 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 = trueContext
StackExchange Code Review Q#139308, answer score: 2
Revisions (0)
No revisions yet.