C++ primer 5 edition: class Message and Folder

Question

I am trying to write the solution for an exercise in C++ primer 5 ed. The program is writing two classes Message and Folder. each Folder has a set of pointers to a Message object and each Message has a set of pointers to Folders in which it exists so I wrote this code:

• For brevity I didn't show the whole code so I'll try to focus on the part may causing problem:

  class Folder;
  class Message
  {
  public:
      Message(const std::string & = "");
      Message(const Message&);
      Message(Message&&);
      Message& operator=(const Message&);
      Message& operator=(Message&&);
      ~Message();
  
      const std::string& getMsg()const;
      void setMsg(const std::string&);
      void  save(Folder&);
      void remove(Folder&);
  
  //private:
      std::string msg_;
      std::set<Folder*> dirs_;
  
      void addToDirs(const Message&);
      void delFromDirs();
      void moveMsgs(Message&);
  
      friend void swap(Message&, Message&);
      friend class Folder;
  };
  
  class Folder
  {
  public:
      void addMsg(Message&);
      void delMsg(Message&);
  
  //private:
      std::set<Message*> msgs_;
  };
  
  Message::Message(const std::string& msg) : 
      msg_(msg)
  {}
  
  Message::Message(const Message& rhs) :
      msg_(rhs.msg_), 
      dirs_(rhs.dirs_)
  {
      addToDirs(rhs);
  }
  
  Message& Message::operator=(const Message& rhs)
  {
      delFromDirs();
      msg_ = rhs.msg_;
      dirs_ = rhs.dirs_;
      addToDirs(rhs);
  
      return *this;
  }
  
  
  void Message::addToDirs(const Message& msg)
  {
      for (auto& fdr : msg.dirs_)
          fdr->addMsg(*this);
  }
  
  Message::~Message()
  {
      delFromDirs();
  }
  
  void Message::save(Folder& fdr)
  {
      fdr.addMsg(*this);
  }
  
  void Message::remove(Folder& fdr)
  {
      fdr.delMsg(*this);
  }
  
  void Message::delFromDirs()
  {
      for (auto fdr : dirs_)
          fdr->delMsg(*this);
  }
  
  void Folder::addMsg( Message& msg)
  {
      msgs_.insert(&msg);
      msg.dirs_.insert(this);
  }
  
  void Folder::delMsg(Message& msg)
  {
      msgs_.erase(&msg);
      msg.dirs_.erase(this);
  }
  
  const std::string& Message::getMsg()const
  {
      return msg_;
  }
  
  void Message::setMsg(const std::string& msg)
  {
      msg_ = msg;
  }
  
  void Message::moveMsgs(Message& msg)
  {
      msg_ = std::move(msg.msg_);
      dirs_ = std::move(msg.dirs_);
  
      for (auto& fdr : dirs_)
      {
          fdr->delMsg(msg);
          fdr->addMsg(*this);
      }
      msg.dirs_.clear();
  }
  
  Message::Message(Message&& rhs) :
      msg_(std::move(rhs.msg_)),
      dirs_(std::move(rhs.dirs_))
  {
      std::cout << "Message's move-ctor\n";
      moveMsgs(rhs);
  }
  Message& Message::operator=(Message&& rhs)
  {
      std::cout << "=(Message&&)\n";
      if (this != &rhs) 
      {
          delFromDirs();
          moveMsgs(rhs);
      }
      return *this;
  }
  
  
  
  int main()
  {
  
      Folder f1, f2, f3;
  
      Message m1("Message m1"),
          m2("Message m2"), m3("Message m3"), m4("Message m4"),
          m5("Message m5");
  
      f1.addMsg(m1);
      f1.addMsg(m2);
      f1.addMsg(m5);
      f2.addMsg(m1);
      f2.addMsg(m4);
      f3.addMsg(m3);
  
  
      cout << "f1.msgs: \n";
      for (auto& msg : f1.msgs_)
          cout << msg->msg_ << endl;
      cout << endl;
  
  
  
  
      std::cout << "\nDone!\n";
  }
  

• The problem is: The code works fine on VC++19 and compiled on wandbox.org using GCC-Head 10 and Clang 10 and works fine but I have an another compiler on my phone called "coding C++" it says:

"process completed (signal 11) press enter".

• I want to know whether delFromDirs() is correctly handling deletion? Because the destructor of Message should remove each folder from its set of folders and each folder from them should remove that message from its set of messages.

Answer 1

You are iterating over dirs_ in delFromDirs and you are calling fdr->delMsg(*this) which does

msgs_.erase(&msg);
msg.dirs_.erase(this);

The line msg.dirs_.erase(this) erases fdr from the dirs_ that you are iterating over. Since this is the element that you are currently iterating over, it will invalidate the current iterator of the range-for loop and then when the loop tries to increment the iterator, it will cause undefined behavior.

Don't remove elements from a container that you are currently iterating over, at least not the current element of a range-for loop. If you need to do that, you can take the return value from erase (which will be a valid iterator to the next element) and reset your loop iterator to continue from there. This then doesn't work with a range-for, though.