Bug 1413

Created attachment 1384 [details]
Several extensions

Created attachment 1385 [details]
TopologyReader and derivatives are no longer Objects

To allow smart pointers, TopologyReader is just inherited from SimpleRefCount<TopologyReader> class

Created attachment 1386 [details]
Making default constructor for TopologyReader::Link public

(necessary for use Link in STL containers)

Hi,

I have a general comment about the patches: I can't figure out exactly what is the intended purpose. I'm not against changes, but I am a bit concerned when the changes are not meant to address a specific case.

Now, the changes you're proposing are not "bad", but it's not clear to me where the code will be reused. The Link class was meant to be an internal implementation for the TopologyRead modules, thus is private. As an internal implementation class, everything is done so to a future TopologyRead implementation change can't affect any other code.

Now, if your changes are so to extend the TopologyRead module itself, the Link class can be left private. If, on the other hand, the changes are so to use the Link class in a completely different module, then the right approach is to design *from scratch* a new class to be put (for example) in the network/utils directory, and have both modules relying on it. The whole point of modules is to avoid changes in one module breaking other modules (as much as possible).

Those are, basically, my concerns. I think we can find a suitable way to fix this point.

T.