开发者

C++: Defined my own assignment operator for my type, now .sort() wont work on vectors of my type?

开发者 https://www.devze.com 2023-02-07 01:40 出处:网络
I have a class (those that have read Accelerated C++ may find this class familiar) defined as follows:

I have a class (those that have read Accelerated C++ may find this class familiar) defined as follows:

class Student_info{
public:
    Student_info() : midterm(0.0), final(0.0) {};
    Student_info(std::istream& is){read(is);};

    Student_info(const Student_info& s);

    ~Student_info();

    Student_info& operator=(const Student_info& s);

    //Getters, setters, and other member functions ommited for brevity

    static int assignCount;
    static int copyCount;
    static int destroyCount;

private:
    std::string name;
    double midterm;
    double final;
    double finalGrade;
    std::vector<double> homework;

};

typedef std::vector<Student_info> stuContainer;


bool compare(const Student_info& x, const Student_info& y);

A function calculator() makes use of objects of this type. As a part of the function, a vector of (already declared) Student_info objects is sorted using the library's generic sort function. My program does not progress past this point (though according to NetBeans no exceptions are thrown and the program exits correctly).

The sort function makes heavy use of the assignment operator of whatever type is held in the container, but I can't seem to find out what's wrong with the one I defined (the program was functioning properly before I defined it). According to Accelerated C++ (or at least this is how I interpreted it), the proper way the assignment operator is supposed to work is to first destroy the left operand, and then construct it again with a value equal to the right operand. So this is my overloaded operator= definition:

Student_info& Student_info::operator=(const Student_info& s)
{
    if(this != &s)
    {
        this->~Student_info();
        destroyCount++;

        *this = s;
    }

    return *this;
}

As you can see, it invokes the Student_info copy constructor, which is defined below:

Student_info::Student_info(const Student_info& s)
{
    name = s.name;
    midterm = s.midterm;
    final = s.final;
    finalGrade = s.finalGrade;
    homework = s.homework;

    c开发者_运维技巧opyCount++;
}

The copy constructor functions correctly, because ommiting the sort statement allows the program to function correctly and yield a copyCount (which is only modified in the copy constructor and operator=) greater than 0.

So what exactly is wrong with my assignment operator? It has to do with the destruction of the calling Student_info object, but I don't know how to rectify it short of NOT destroying it.

(And by the way, the creation of the copy constructor, destructor, and assignment operator are called for by an exercise in Accelerated C++... I realize that the synthesized versions of these functions would obviously suffice for my class)


No, no, no. It's not supposed to work that way at all. Your current assignment operator destroys the object it's called on, then calls itself (oh hey, infinite recursion) on a destroyed object (oh hey, undefined behaviour). You are not supposed to destroy the existing object. At all. And this code *this = s does not invoke any constructor at all, it invokes the assignment operator- which is what you're just defining. A copy constructor call would look like new (this) Student_info(s);. This is a known pattern and it's terrible in many, many ways. If you have a book that's recommending it, throw it in the bin.

The assignment operator is supposed to copy the data from the right-hand-side into the left-hand-side. The easiest way to do that in most cases is just to copy each data member. The semantics of this operator do not involve destroying anything. Anyone using this operator has the right to expect that there is no destruction of any Student_info objects going on.

Just call the member's existing assignment operators and then implement whatever additional logic you need.


*this = s;

this is endless recursion. it's not copy-constructor, it's assignment operator


struct Student_info {
  Student_info& operator=(Student_info other) {
    swap(*this, other);
    return *this;
  }

  friend void swap(Student_info &a, Student_info &b) {
    using std::swap;
    #define S(N) swap(a.N, b.N);
    S(name)
    S(midterm)
    S(final)
    S(finalGrade)
    S(homework)
    #undef S
  }

private:
  std::string name;
  double midterm;
  double final;
  double finalGrade;
  std::vector<double> homework;
};

Getters, setters, and other member functions ommited for brevity

If you have public getters and setters which are just boilerplate, consider marking the corresponding data members as public instead.

0

精彩评论

暂无评论...
验证码 换一张
取 消