I am trying to sort a vector of class instances in C++

Question

I have defined a class, and I have a vector of those class instances. I want to sort the vector by one of the properties of the class. I overrode the operator< so it would know how to sort it. My understanding is that the operator< is the default sort method. It seems like I'm missing something simple. Below is a stripped-down simplified version of what I am trying to do. Any ideas?

#include <iostream>
#include <string>
#include <vector>
#include <algorithm>


class C {
    std::string name;

public:
    C() {};

    C(std::string s) {
        name = s;
    }

    const std::string getName() {
        return name;
    }

    bool operator<(const C& x) const {
        return (name > x.name);
    }
};


int main() {
    std::vector<C*> v;
    C* c;

    c = new C("Tom");
    v.push_back(c);
    c = new C("Jane");
    v.push_back(c);
    c = new C("Dick");
    v.push_back(c);
    c = new C("Harry");
    v.push_back(c);

    
    std::sort(v.begin(), v.end());
    for (int i = 0; i < v.size(); i++) {
        std::cout << v[i]->getName() << std::endl;
    }
}

Every time I run this, they come back in a random order. I suspect my operator< is not being used and they are just getting sorted by their addresses in memory.

score 1 · Answer 1 · answered Jan 25 '23 at 17:26

You don't have a vector of class instances, you have a vector of pointers. Therefore, it's sorting the pointers, based on the ordering of the pointers.

The solution is, as usual: do not use pointers.

int main() {
    std::vector<C> v;
    C c;

    c = C("Tom");
    v.push_back(c);
    c = C("Jane");
    v.push_back(c);
    c = C("Dick");
    v.push_back(c);
    c = C("Harry");
    v.push_back(c);

    
    std::sort(v.begin(), v.end());
    for (int i = 0; i < v.size(); i++) {
        std::cout << v[i].getName() << std::endl;
    }
}

Notably, you can also simplify the first part drastically, and it will also run faster.

std::vector<C> v {"Tom", "Jane", "Dick", "Harry"};

In the future: Never use new. If you think you need new T, then use std::make_unique<T> instead.

I am trying to sort a vector of class instances in C++

1 Answers1