snippetrubyMinor
Insertion sort in ruby
Viewed 0 times
insertionsortruby
Problem
The code is correct, passes all testcases, I am looking for input on cleaning it, readability etc
def insertionSort(a)
return if a.nil? || a.empty? || a.length == 1
sort_index = 0
for i in 1..a.length-1 do
for j in 0..sort_index do
next if a[i] > a[j]
temp = a[i]
sort_index.downto(j) do |n|
a[n+1] = a[n]
end
a[j] = temp
end
sort_index +=1
end
a
endSolution
Ruby tends to use two spaces for indents, rather than four.
Avoid
with
and the rest of the code is (probably) identical. For this case, however, since you're iterating over a range of integers, I'd suggest using
Or, if you feel like you really should have
Use more informative variable names --
I don't see a difference between
You should return
Sorts in Ruby should use `
Avoid
for; it's very uncommon in Ruby and, while technically correct, doesn't make full use of Ruby's functionality. In the general case, you can replacefor a in b dowith
a.each do |b|and the rest of the code is (probably) identical. For this case, however, since you're iterating over a range of integers, I'd suggest using
upto:1.upto(a.length-1) do |i|
0.upto(sort_index) do |j|Or, if you feel like you really should have
for a in b or b.each do |a|, you can use 1...a.length to exclude the last element, instead of subtracting one.Use more informative variable names --
i and j might make sense to you now, but it's rather difficult to understand if you don't already know how insertion sort works.I don't see a difference between
sort_index and i - 1, though I haven't had a chance to run it and test it yet.You should return
a if a.length == 1, not nothing, since sorting a 1-item array is still sorting. Because the sort won't do anything, you can just remove the check entirely from the first if and forget about it.Sorts in Ruby should use `
instead of because is the universal comparison operator. See the docs for Object#` for more information.Code Snippets
for a in b doa.each do |b|1.upto(a.length-1) do |i|
0.upto(sort_index) do |j|Context
StackExchange Code Review Q#100511, answer score: 5
Revisions (0)
No revisions yet.