Discussion on: Non isomorphic C++ refactoring

View post

Replies for: OK, I'll bite. RAII uses a clean-up action which occurs at the end of the object's lifetime. The lifetime of the Door in your refactored version is...

Dimitri Merejkowsky • Aug 6 '17 • Edited

That's a good point. For instance with:

class Safe {

void dust() {
    std::cout << "Removing dust from the safe" << std::endl;
  }
}

int main() {
  Safe safe;
  safe.open(combination, key);
  safe.putDocuments(documents);
  safe.dust();
}

You will get:

Opening door
Putting passport in safe
Putting ID card in safe
Removing dust from the safe
Closing door

Which is probably not want you want. That's the issue with contrived examples :)

We can make it work by having:

Get rid of Door destructor
Add an explicit .close() method in the Safe class though
Store the state of the safe inside a member of the class
Ensure the safe is closed in the destructor.

I'll update the article, thanks.

Dave Cridland • Aug 6 '17

I'd make safe.open(...) return a Door as a guard. Maybe:

class Door {
  bool m_open = false;
  Safe & m_safe;
  public:
    Door(Door && door) : m_open(door.m_open), m_safe(door.m_safe) {
      door.m_open = false;
    }
    Door(Safe & safe) : m_safe(safe) {}
    void opened() {
      m_open = true;
    }
    ~Door() {
      if (m_open) safe.close(*this);
    }
};

class Safe {
  // ...
  Door open( ... ) {
    Door door(*this);
    // Do the open;
    door.opened();
    return std::move(door);
  }
  // ...
};

But I am addicted to RAII, it must be said.