Hi Atul,

I happen to have a copy of C++ Coding Style and looked at Item 42. The summary says:

“Don’t volunteer too much: Avoid returning handles to internal data managed by your class, so clients won’t uncontrollably modify state that your object thinks it owns.”

The example from the book:

class String {
  char* buffer_;
public:
  char* GetBuffer() const {return buffer_;}  // bad: should return const char*
  // …
};

I don’t see how your patch addresses this issue! Generally it would be very rarely that we return a pointer or reference to the internal representation. We do return copies but that’s quite fine.

Now regarding the patch, your parse() function is NodeAddress is better than my implementation in one of the ctors. But why do you want it public? Let’s keep it private and have the parsing ctor call it. If you need to parse a new string, then create a new object and copy it:

NodeAddress address;
…
address = NodeAddress (ipPortString);

It’s better style to avoid operations with side effects. That’s why for example QByteArray::left() does not modify the objects but instead returns a new one. This design concept is used pretty much and in my opinion pretty well in Qt.

Regarding PackedHostCaches, your version of readData() is better ;-). My only suggestion would be that readProperty() be renamed to readHostCache() because you have a ‘\n’ delimited list of hosts caches of the form address&properties.

What is the problem with Private being used as a pure struct? It’s used to store the private data and allow for easy addition of reference counting for example. In my opinion writing two constructors for Private is overkill and due to its design it is NOT subject to feature envy. I think we already discussed private data and feature envy but we can reopen the discussion.

Best regards,

Peter

atul wrote:Dear all

C++ Coding Standards by Herb Sutter and Andrei Alexandrescu have this important guideline…

Much of our code seems to violate this… ( I think we briefly discussed this ). I always thought this a major design concern in our code.

There is a lot of scope for removing feature envy too…

Here is the patch and a general idea….

— cheers atul