Insertion sort in ruby

Question

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 
end

• A H K: stackoverflow.com/a/38858611/1623261

Answer 1

Ruby tends to use two spaces for indents, rather than four.

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 replace

for a in b do

with

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 < or > because <=> is the universal comparison operator. See the docs for Object#<=> for more information.

Answer 2

1. Method names should have words separated by underscores instead of using camelCase to follow standard naming conventions: def insertion_sort(a)

2. I've got a love-hate relationship with single letter variable names. In some contexts, such as this method, it might be OK, because the method name could infer what a means, but consider changing the argument name to array_to_sort or sort_array. This isn't necessarily an official naming convention, but I've seen many Ruby developers use arr in these situations, though arguably arr isn't much better than a.

3. The return if ... line will return nil if the array is nil, doesn't have any values or contains one value. Consider this scenario:

   arr = [1]
   arr = insertion_sort(arr)
   

   This will result in arr being set to a nil value, which doesn't seem right. This would be better written as:

   def insertion_sort(a)
    raise ArgumentError, "Cannot pass a nil value" if a.nil?
    return a if a.empty? || a.length == 1
    ...
    a
   end
   

   This keeps the return value of this method consistent. Additionally, passing a nil value is an "exceptional" condition. You cannot insert or sort a nil value, so an ArgumentError should be thrown here.

   After that, if the array is empty or contains just one element, just return the array early before performing the insertion sort.

• anon: A bit late, but I think arr is significantly better than a. I mean, it sounds all pirate-y.