C++ Array not sorting properly

Question

I wanted to sort nuggets by unit price, so I made a class Nugget with the variables quant, and price, then made a double unit, which is price/quant. The nuggets come in 4, 6 and 9 packs

When I input 10, 10, and 10, for the price of each, I should get a sorted array of 9pack, 6pack and then a 4pack, because 10/9 is less than 10/6 and 10/4. But thats not the case.

#include <iostream>
using namespace std;

class Nugget {
    public:
        int price;
        int quant;
        double unit;

        Nugget(int price, int quant) {
            this->price = price;
            this->quant = quant;
            this->unit = price/quant;
        }
};

int main(){    
    int n4,n6, n9;
    cin >> n4 >> n6 >> n9;

    Nugget* nuggetArr[3] = {new Nugget(n4,4), new Nugget(n6,6), new Nugget(n9,9)};

    for (int i = 0; i < 3; ++i) {
        for (int j = 0; j < 3; ++j) {
            if (nuggetArr[j]->unit > nuggetArr[i]->unit) {
                Nugget* temp = nuggetArr[i];
                nuggetArr[i] = nuggetArr[j];
                nuggetArr[j] = temp;    
            }
        }

        for (int j = 0; j < 3; j++)
            cout << nuggetArr[j]->quant << ' ';
        cout << endl << endl;
    }

    for (int i = 0; i<3; ++i)
        cout << nuggetArr[i]->quant << ' ';

    return 0;
};

You are relying on the constructor to calculate the `unit` value that you are going to sort on. You are only printing the `quant` value, not the `unit` value. How do you know if it's correct? One suggestion would be to set the value directly after constructing the objects. Another would be to print out each member of each object _before_ and _after_ sorting. — Tim Randall, Dec 06 '18 at 14:22
Welcome to StackOverflow. One thing you might want to read about is that [`using namespace std;` is not recommended](https://stackoverflow.com/q/1452721) *(though it would be fine to say `#include ` then `using std::cin;` and `using std::cout;`)* Another thing to know is that while semicolons are required after class definitions you don't have to put semicolons after function definitions... be sure to [turn up your warning levels](https://stackoverflow.com/questions/399850/best-compiler-warning-level-for-c-c-compilers), and ideally use -Werror to force you to pay attention to them! — HostileFork says dont trust SE, Dec 06 '18 at 14:26
Are you trying to implement a specific sorting algorithm? This algorithm simply isn't a correct implementation of sorting, try looking at [bubble sort](https://en.wikipedia.org/wiki/Bubble_sort) or [insertion sort](https://en.wikipedia.org/wiki/Insertion_sort), or use [`std::sort`](https://en.cppreference.com/w/cpp/algorithm/sort) rather than rolling your own implementation. — hnefatl, Dec 06 '18 at 14:26

Frane · Accepted Answer · 2018-12-06T15:33:07.080

What sorting algorithm you want to use?

If you swap nuggetArr[i] for nuggetArr[j] you should ensure that i < j for ascending or i > j for descending order. For example:

    for (int i = 0; i < 3; ++i) {
        for (int j = i + 1; j < 3; ++j) {
        if (nuggetArr[j]->unit > nuggetArr[i]->unit) {
            Nugget* temp = nuggetArr[i];
            nuggetArr[i] = nuggetArr[j];
            nuggetArr[j] = temp;    
        }
    }

Your code moves around objects (nuggets) not keeping some order. For example you swap nuggetArr[1] for nuggetArr[2] and then nuggetArr[2] for nuggetArr[1].

FYI this is similar to Selection sort.

Lala5th · Answer 2 · 2018-12-06T14:39:10.363

-1

C++ uses a wierd way of casting here. It uses integers until assignement, so the unit value will be an integer even though you used double. If you make the following changes you will find that it will work (Please note that this is just to illustrate the point and is not good to use this because of precision):

...
double price;
double quant;
...
this->unit = this->price/this->quant;
...

Hope this helps

To be more precise as a note if a,b are integers then a/b will be an integer by default and after the division will it only be cast as a double.

edited Dec 06 '18 at 14:39

answered Dec 06 '18 at 14:33

Lala5th

1,137
7
18

3

A more common technique is simply to promote one of the values before doing the division: `this->unit = (double)price/quant;` The compiler will then promote the other value automatically and perform a division using `double` precision rather than integer math – Tim Randall Dec 06 '18 at 14:42

C++ Array not sorting properly

2 Answers2