My own method for sorting numbers in array

Question

I've written some method for sorting asc and desc numbers in array. All I want to know is this good logic, and is there a space for improving this code?

-(NSArray *)sortHanded:(NSArray *)inputArray ascending:(BOOL)ascending {
    NSMutableArray *inputMutableArray = [inputArray mutableCopy];
    NSMutableArray *sortedArray = [NSMutableArray new];
    NSNumber *min;
    NSNumber *max;
    if (ascending == YES) {

        //starting while
        while (inputMutableArray.count > 0) {


            //starting switch
            switch (inputMutableArray.count) {

                    //case 0
                case 0:

                    break;

                    //case 1
                case 1:
                    [sortedArray addObject:[inputMutableArray objectAtIndex:0]];
                    [inputMutableArray removeObject:[inputMutableArray objectAtIndex:0]];
                    break;

                    //starting default switch
                default:
                    for (NSNumber *temp in inputMutableArray) {
                        if (min == nil) {
                            min = temp;
                        }

                        if (min > temp) {
                            min = temp;
                        }

                    }

                    [sortedArray addObject:min];
                    [inputMutableArray removeObject:min];
                    min = nil;
                    break;
                    //end default
                    //end start switch
            }

            //ending while
        }

    }
    else {
        //starting while
        while (inputMutableArray.count > 0) {


            //starting switch
            switch (inputMutableArray.count) {

                    //case 0
                case 0:

                    break;

                    //case 1
                case 1:
                    [sortedArray addObject:[inputMutableArray objectAtIndex:0]];
                    [inputMutableArray removeObject:[inputMutableArray objectAtIndex:0]];
                    break;

                    //starting default switch
                default:
                    for (NSNumber *temp in inputMutableArray) {
                        if (max == nil) {
                            max = temp;
                        }

                        if (max < temp) {
                            max = temp;
                        }

                    }

                    [sortedArray addObject:max];
                    [inputMutableArray removeObject:max];
                    max = nil;
                    break;
                    //end default
                    //end start switch
            }

            //ending while
        }

    }
        //returning sorted array
    return sortedArray;
}

Answer 1

There is a big flaw in your program: This

if (min > temp)

compares the addresses of the number objects, not their values. The correct way is

if ([min compare:temp] == NSOrderedDescending)
// ...
if ([max compare:temp] == NSOrderedAscending)

You probably didn't notice the problem because "small" numbers are represented as "tagged pointers", but with

NSArray *array = @[@40000000000000, @300000, @10000000000000, @16];

your code gave a wrong result in my test.

---

Resetting min after the loop seems a bit counter-intuitive to me. I would do that at the beginning, this also allows to restrict the scope of that variable to where it is needed. Also the second if should be an else-if:

NSNumber *min = nil;
for (NSNumber *temp in inputMutableArray) {
    if (min == nil) {
        min = temp;
    } else if ([min compare:temp] == NSOrderedDescending) {
        min = temp;
    }
}

Of course you could contract the if statement to

    if (min == nil || [min compare:temp] == NSOrderedDescending) {
        min = temp;
    }

that is a matter of taste.

---

You could get rid of this loop by using Key-Value Coding to determine the minimum value in the array:

NSNumber *min = [inputMutableArray valueForKeyPath:@"@min.self"];

But note that this can be significantly slower for large arrays (compare Finding the smallest and biggest value in NSArray of NSNumbers).

---

Having almost identical code for the two cases (ascending/descending sort) is not necessary. If you define

NSComparisonResult sortOrder = ascending ? NSOrderedAscending : NSOrderedDescending;

at the beginning, then both cases can be treated with

while (inputMutableArray.count > 0) {
    NSNumber *first = nil;
    for (NSNumber *temp in inputMutableArray) {
        if (first == nil || [temp compare:first] == sortOrder) {
            first = temp;
        }
    }

    [sortedArray addObject:first];
    [inputMutableArray removeObject:first];
}

Answer 2

You don't need to include comments for stuff that's self evident from the code and you do it a bunch here.

//case 0
case 0:

Comments should add context, background information or explain complicated functions. Better would be a comment saying what case 0 means. Removing the whitespace and all your comments makes this whole block cleaner and should make the indentation clear enough that you don't need to signal when a loop ends:

if (ascending == YES) {
    while (inputMutableArray.count > 0) {
        switch (inputMutableArray.count) {
            case 0:
                break;

            case 1:
                [sortedArray addObject:[inputMutableArray objectAtIndex:0]];
                [inputMutableArray removeObject:[inputMutableArray objectAtIndex:0]];
                break;

            default:
                for (NSNumber *temp in inputMutableArray) {
                    if (min == nil) {
                        min = temp;
                    }
                    if (min > temp) {
                        min = temp;
                    }
                }

                [sortedArray addObject:min];
                [inputMutableArray removeObject:min];
                min = nil;
                break;
        }
    }

Answer 3

Many sorting libraries will sort an array in place instead of allocating a new array and returning it. They will require a mutable array to be passed in.

You test the count of elements in the input array and then have special cases for when it is almost empty.

It's clearer if you test if there are more than 1 elements in the inputMutableArray and put the special cases after the loop:

while (inputMutableArray.count > 1) {
    for (NSNumber *temp in inputMutableArray) {
        if (min == nil) {
            min = temp;
        }
        if (min > temp) {
            min = temp;
        }
    }

    [sortedArray addObject:min];
    [inputMutableArray removeObject:min];
    min = nil;

}

//works for both empty and non-empty inputMutableArrays
[sortedArray addObjectsFromArray:inputMutableArray];
[inputMutableArray removeAllObjects];

Your implementation requires Automatic Reference counting. If you have disabled it (for whatever reason) then you leak the inputMutableArray; you should release the things you allocate and don't return. That is add [inputMutableArray release] before the return.

Answer 4

What if our array has strings in it? Does ascending still make sense? Maybe. Does it mean alphabetically ascending? Or does it mean ascending in length?

What if our array holds CLLocation objects? Are we sorting by latitude ascending? Longitude ascending? Altitude ascending? Distance from (0,0) ascending?

What if our array holds NSArray objects? What if it holds NSDictionary objects?

There are a plethora of problems here. Your formula, first of all, as Martin R's answer points out, is flawed in that it is comparing pointer values and not the actual contents of any of the objects in its array.

The naïve fix is to change your < operators to use the compare: method. This also has problems. If we're not using Xcode 7+ or if we want to use any legacy array, we can't use Objective-C generics, so we can't depend on some sort of particular object being in the array, nor can we rely on all of the objects in the array conforming to a protocol that declares the compare: method. What's worse, if we rely on compare:, we box ourselves in to a particular implementation. How do you, for example, use compare: on CLLocation objects? What if you want to sort some CLLocation objects based on distance from a particular point? Or what if you want to sort NSDate objects based on time since a particular point in time (3 minutes in the future is closer to now than 5 minutes in the past, but using compare will result in 5 minutes in the past being sorted before 3 minutes in the future).

This is exactly why the Apple sorting methods expect a "comparator" block argument. The sorting method only cares about the algorithm itself, and it relies on the result of running two objects through the comparator to determine the order they should be sorted in.

So, for example, if dateArray is an array of NSDate objects, and I want to sort it from earliest to latest date, I'd do something like this (using Apple's built-in method):

NSArray *sortedDates = [dateArray sortedArrayUsingComparator:^(id a, id b) {
    return [a compare:b];
}];

If I want to sort in the reverse order, I can do the same and simply flop a and b:

NSArray *sortedDates = [dateArray sortedArrayUsingComparator:^(id a, id b) {
    return [b compare:a];
}];

Or if I want to do something trickier, and sort them based on how far they are from now, I can also do that (still using the same method):

NSArray *sortedDates = [dateArray sortedArrayUsingComparator:^(id a, id b) {
    NSTimeInterval aFromNow = abs(a.timeIntervalSinceNow);
    NSTimeInterval bFromNow = abs(b.timeIntervalSinceNow);

    if (a < b) {
        return NSOrderedAscending;
    } else if (b < a) {
        return NSOrderedDescending;
    } else {
        return NSOrderedSame;
    }
}];

And now without modifying the method or algorithm or making any assumptions about what or how the caller is sorting, my algorithm works for sorting their array in the way they wanted it sorted.

All you have to do internally is compare the two objects you're trying to sort using the comparator block that has been passed in. That will tell you in which order they belong, and will allow you to sort things in a million different ways with the same algorithm. And it's type-agnostic--it can sort anything.