1

I have an array of structs for products that I am trying to sort by name, type, price, and quantity. Name and type work, but price and quantity aren't working. My code is:

else if (sort == sortByPrice)
{
    for (int i = 0; i < numProducts; i++)
    {
        int smallPosition = i;
        for (int x = i + 1; x < numProducts; x++)
        {
            if (list[i].price > list[x].price)
            {
                smallPosition = x;
            }
        }

        temp = list[i];
        list[i] = list[smallPosition];
        list[smallPosition] = temp;
    }

}
else if (sort == sortByQty)
{
    for (int i = 0; i < numProducts; i++)
    {
        int smallPosition = i;
        for (int x = i + 1; x < numProducts; x++)
        {
            if (list[i].qty > list[x].qty)
            {
                smallPosition = x;
            }
        }

        temp = list[i];
        list[i] = list[smallPosition];
        list[smallPosition] = temp;
    }
}

Can anyone tell me why it doesn't work/how to fix it?

2
  • You seem to be assuming that your inner "x" loop finds the smallest remaining item. It doesn't--it finds the highest-indexed item that happens to be smaller than the i-th item, but might be larger than some others. Commented Apr 4, 2018 at 0:23
  • if (list[i].price > list[x].price) should be if (list[x].price < list[smallPosition].price) . That should be enough. Otherwise, if you're not interested in your Selection Sort implementation, the answer below is a Bubble Sort implementation and should work. Commented Apr 4, 2018 at 0:59

3 Answers 3

3

Following up on Lee Daniel Crocker's comment, you should dynamically compare with the value at smallPosition instead of i so that it will always point to the smallest remaining item:

    int smallPosition = i;
    for (int x = i + 1; x < numProducts; x++)
    {
        if (list[smallPosition].price > list[x].price)
        {
            smallPosition = x;
        }
    }
Sign up to request clarification or add additional context in comments.

1 Comment

And a just a small improvement: before swap the elements check if (smallPosition != i), then swap if true. This will prevent your swap operations to be done in case your array is already sorted.
0

You should move the swap code inside the if statement:

for (int i = 0; i < numProducts; i++)
{
    for (int x = i + 1; x < numProducts; x++)
    {
        if (list[i].price > list[x].price)
        {
            temp = list[i];
            list[i] = list[X];
            list[X] = temp;
        }
    }

}

1 Comment

Even tough this should work, it's nice to point out that this is a BubbleSort implementation, and OP's original attempt was to create a SelectionSort implementation.
0

Just use bubble sort.

In the bubble sort you swap the values of the current value is bigger than the next one, if you do this action until you get to the end of the array then the array will be sorted.

1 Comment

I'm downvoting this answer for two reasons: 1) That's exactly what @bruceg answers' doing. 2) Also, OPs original idea is to implement a SelectionSort, as I commented on bruceg answer. Even tough they're both worst-case O(n²) algorithms, when it comes to the avg. case, SelectionSort is fairly better than Bubble (read: en.wikipedia.org/wiki/…)

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.