Sort vector of object receive stack overflow exception c++

Question

I'm trying to sort a vector of Student objects by an attribute:

class Student
{
    private:
        std::string nume;
        int an;
        std::list<Curs> cursuri;
        ...
    public:
        Student();
        Student(std::string nume, int an);
        virtual ~Student();
        ...
};

with these sorting method comparatator:

bool Student::sortByMedie(const Student& a, const Student& b)
{
    return a.medie < b.medie;
}

void sortStudenti(std::vector<Student> studenti) {

    std::sort(studenti.begin(), studenti.end(), Student::sortByMedie);

    for (auto student : studenti) {
        student.afisare();
    }
}

But I am encounter a problem with a stack overflow exception when the sort method is called:

The thread 0x4f6c has exited with code 0 (0x0). Exception thrown at 0x776CBA3E (ntdll.dll) in LAB3.exe: 0xC00000FD: Stack overflow (parameters: 0x00000001, 0x01002FF0). Unhandled exception at 0x776CBA3E (ntdll.dll) in LAB3.exe: 0xC00000FD: Stack overflow (parameters: 0x00000001, 0x01002FF0).

I'm assuming the problem is somewhere in a reallocation of vector size in memory. If I browse through the stack trace beyond the memory allocation functions, the last function of my own code (i.e. not standard library) is a Curs copy constructor called by a swap between two Cusr elements, that is invoked by Curs::operator=

This is the creation of vector:

    std::vector<Student> studenti;
    auto student1 = Student("gigel marian", 3);
    student1.addCursuri(generateCoursList());
    auto student2 = Student("gigel marian2", 3);
    student2.addCursuri(generateCoursList());
    auto student3 = Student("gigel marian3", 3);
    student3.addCursuri(generateCoursList());
    auto student4 = Student("gigel marian4", 3);
    student4.addCursuri(generateCoursList());
    auto student5 = Student("gigel marian5", 3);
    student5.addCursuri(generateCoursList());

    studenti.push_back(student1);
    studenti.push_back(student2);
    studenti.push_back(student3);
    studenti.push_back(student4);
    studenti.push_back(student5);

In first place I tried with this method:

void sortStudenti(std::vector<Student> studenti) {
    struct studentCompare
    {
        bool operator()(Student const& a, Student const& b)
        {
            return a.getMedie() > b.getMedie();
        }
    };

    std::sort(studenti.begin(), studenti.end(), studentCompare());

    for (auto student : studenti) {
        student.afisare();
    }
}

but I got some const access errors, so I tried in another way.

Edit: additional code

The full code is available on github

Answer 1

When sort() tries to swap Student elements, it makes temporary copies of Student elements. Since you do not specify anything else, it's the default member-by-member copy that will be performed.

In your Student class, you have a list of Curs. The list is simlarly copied with the Curs elements it contains. But for Curs, you have defined your own assignement operator:

Curs& Curs::operator=(Curs arg) noexcept
{
    std::swap(*this, arg);
    return *this;
}

The code that is behind the swap() generated makes you copy Curs again, which will call the swap and the Curs again, .... and so on until the stack overflows or the memory is out.

By the way, I see that you have created this operator to circumvent the limitations behind the const members that Curs class contains. Tricking the compiler in this way to change a const element is undefined behavior. So get rid of constness for members that need to be copied.

Just get rid of this (wrongly implemented) operator and the constness, and it will work.

---

Additional recommendations

This problem is indeed a complex one to handle with just some extracts of code. Especially when the problem is not where we think it is. So I can only recommend you in future:

Exploit the error information you have to its best extent

• Post your complete error message in your question.
• In the case of a stack overflow: browse through the stack trace until you find some code of yours, to see in which part of your code it occurs. This helps to narrow down the research.
• If you browse a little bit more, you can also check if there is an endless recursion (the most frequent reason for a stack overflow with small data amount): a very long succession of almost identical calls is a typical symptom.

Reduce the risk of errors

• Before starting to build more complex classes, make tests to ensure that the underlying classes work as expected.
• Even if you have not enough time to write extensive tests, you should at least try every class member function. If you would have done a test that just checks if the Curs::operator= works, you would have saved you a lot of additional experiments ;-)

Answer 2

You can pass your vector by reference and without changing your objects by passing them to the loop by const reference, as follows.

But make sure that the member function afisare is const

void sortStudenti(std::vector<Student>& studenti) {
    struct studentCompare
    {
        bool operator()(Student const& a, Student const& b)
        {
            return a.getMedie() > b.getMedie();
        }
    };


    std::sort(studenti.begin(), studenti.end(), studentCompare());

    for (const auto& student : studenti) {
        student.afisare();
    }
}

But I think there is another reason for the exception. You should have checked your class definition