An acceptable way of sorting List

Question

I had a problem where I needed to find the largest amount of passengers in multiple cars/vans etc, so I got the totals from each vehicle and added them to a list, I then created this method to sort the list...

static List<int> SortList(List<int> tobesorted)
{
    var j = 0;
    int JP = 0;

    int value1 = Convert.ToInt32(tobesorted[j]);
    int value2 = Convert.ToInt32(tobesorted[JP]);

    for (var i = 0; i < tobesorted.Count-1; i++)
    {
        for (j = 0; j < tobesorted.Count-1; j++)
        {
            JP = (j + 1);
            value1 = Convert.ToInt32(tobesorted[j]);
            value2 = Convert.ToInt32(tobesorted[JP]);

            if (value2 > value1)
            {
                tobesorted[j] = value2;
                tobesorted[JP] = value1;
            }
        }
        j = 0;
        JP = 0;
    }

    return tobesorted;
}

I don't understand is this is acceptable, and if not what methods are? It seems quick enough, I know of sorting algorithms, but haven't ever been forced to use any.

Answer 1

I needed to find the largest amount of passengers

If you need just the largest amount, you don't need to sort the whole list (which is slow*), you can instead walk though the list, remember the largest amount found so far and update that as you iterate the list.

LINQ already contains a method that does just that, called Max().

---

* Sorting is \$\mathcal{O}(n \log n)\$ with a good algorithm, or \$\mathcal{O}(n^2)\$ with a trivial algorithm like the one you used; finding maximum is just \$\mathcal{O}(n)\$.

Answer 2

int value1 = Convert.ToInt32(tobesorted[j]);
int value2 = Convert.ToInt32(tobesorted[JP]);

tobesorted is a List<int>. Hence, all its items are int. int is a C# language alias / shortcut for System.Int32 - therefore, the calls to Convert.ToInt32 are redundant and can be safely removed.

Who's JP? (i.e. use meaningful variable names ;)

List<T> already has a built-in Sort method that should do the trick...

Answer 3

j is initialized every time the inner loop executes. So there is no need to define it in the outer scope. You can remove j in the outer scope and define the inner loop as

for (int j = 0; j < tobesorted.Count-1; j++)
    {
       // ...
    }

With this change there is no need to reset the value of j after the completion of inner loop.

Since tobesorted is a reference type so the method definition can be changed to

static void SortList(List<int> tobesorted)
{

Answer 4

Mat's Mug is completly right. You shouldn't be calling Convert.ToInt32 in your method since you already know that your list items are of type int. Here is your sort alghorithm, the selecting sort algorithm.

static List<int> SortList(List<int> list)
{
    for (int i = 0; i < list.Count-1; i++)
    {
        int idxMin = i;
        for (int j = i+1; j < list.Count; j++)
        {
            if (list[j] < list[idxMin])
            {
               idxMin = j;
            }
        }
        int aux = list[i];
        list[i] = list[idxMin];
        list[idxMin] = aux;
    }

    return list;
}

and a test case

List<int> list = new List<int>(){17,7,3,4,11,15,23,5,10,1};
foreach(int i in SortList(list)){
    Console.Write("{0}, ", i);
}

Answer 5

It looks to me that you're trying to scratch your ear by reaching around your elbow. Assuming you have a class representing the Vehicle, something like this:

enum VehicleType
{
    None,
    Van,
    Sedan,
    HatchBack,
    PickUp
}
public class Vehicle
{
    public VehicleType Type = VehicleType.None;
    public int MaxPassengers = 0;
    public int NumPassengers = 0;
}

And a list of these vehicles:

List<Vehicle> vehicles = new List<Vehicle>()
{
    new Vehicle{Type =  VehicleType.Van, MaxPassengers = 8, NumPassengers = 5},
    new Vehicle{Type = VehicleType.Sedan, MaxPassengers = 5, NumPassengers = 3},
    new Vehicle{Type = VehicleType.PickUp,MaxPassengers = 3, NumPassengers = 1},
};

The highest number of passengers would be:

int MostPassengers = vehicles.Max(x => x.NumPassengers);

The vehicle list sorted by number of passengers would be something like this:

List<Vehicle> SortedByPassengers = vehicles.OrderBy(x => x.NumPassengers);

Every vehicle at the bottom of the list with the same number of passengers will be the highest number of passengers.