6

I was reading about copy-control and came across the following sample in the book C++ Primer (chapter 13.4).

My question is about the remove_from_Folders(); inside copy assignment operator:
If we firstly do remove_from_Folders();, in the case of self assignment, doesn't its folders.clear(); clear the data member of rhs and cause a failure? 

Message& Message::operator=(const Message &rhs)
{
 // handle self-assignment by removing pointers before inserting them
 remove_from_Folders();  // update existing Folders
 contents = rhs.contents; // copy message contents from rhs
 folders = rhs.folders;  // copy Folder pointers from rhs
 add_to_Folders(rhs);  // add this Message to those Folders
 return *this;
}

// remove this Message from the corresponding Folders
void Message::remove_from_Folders()
{
 for (auto f : folders) // for each pointer in folders
     f->remMsg(this);  // remove this Message from that Folder
 folders.clear();
}

The class is defined as:

class Message {
 friend class Folder;
public:
 // folders is implicitly initialized to the empty set
 explicit Message(const std::string &str = ""):
 contents(str) { }
 // copy control to manage pointers to this Message
 Message(const Message&);  // copy constructor
 Message& operator=(const Message&); // copy assignment
 ~Message();  // destructor
 // add/remove this Message from the specified Folder's set of messages
 void save(Folder&);
 void remove(Folder&);
private:
 std::string contents;  // actual message text
 std::set<Folder*> folders; // Folders that have this Message
 // utility functions used by copy constructor, assignment, and destructor
 // add this Message to the Folders that point to the parameter
 void add_to_Folders(const Message&);
 // remove this Message from every Folder in folders
 void remove_from_Folders();
};
Toby Speight
  • 27,591
  • 48
  • 66
  • 103
modeller
  • 3,770
  • 3
  • 25
  • 49
  • 2
    Yes, it appears that the assignment operator will do just what you say in the event of a self-assignment. Looks like a bug to me, the source comment notwithstanding. – John Bollinger Mar 27 '15 at 18:57
  • 1
    The easy way; create a temporary copy and swap (which is exception safe, also) –  Mar 27 '15 at 19:02
  • 1
    take a look at this: http://stackoverflow.com/questions/3279543/what-is-the-copy-and-swap-idiom – NathanOliver Mar 27 '15 at 19:30
  • This seems like a good candidate for [copy and swap](http://stackoverflow.com/questions/3279543/what-is-the-copy-and-swap-idiom) – M.M Jul 05 '15 at 07:31
  • I'm reading the same book and just got to the copy control chapter. I was asking myself the same question.. so it's a error isn't it? – Luca Sep 10 '15 at 13:38
  • I'm reading the chapter currently and immediately started to search for this. The issue was introduced in the first place during the *second print* of the book and is explained in the errata. From my point of view they have broken the code and not fixed it: http://ptgmedia.pearsoncmg.com/images/9780321714114/errata/9780321714114_errata_10-31-12.html – Peter Aug 16 '18 at 13:42

1 Answers1

1

https://github.com/Mooophy/Cpp-Primer/tree/master/ch13

the sampe code listed above is more appropriate for handling operator=(self)

Message& Message::operator=(const Message &rhs) {
    if (this != &rhs) { // self-assigment check is necessary
                        // while remove_from_Folders() do folders.clear()
        remove_from_Folders();    // update existing Folders
        contents = rhs.contents;  // copy message contents from rhs
        folders = rhs.folders;    // copy Folder pointers from rhs
        add_to_Folders(rhs);      // add this Message to those Folders
    } 
    return *this;
}
wkliang
  • 31
  • 3
  • Can somebody explain why this answer was downvoted? It is short, self explanatory and adding only one line of code, if someone wants to keep *folders.clear()* in *remove_from_Folders()* this works and keep working even when further modifications to the class *Message* complicate the situation. – Peter Aug 16 '18 at 13:53