Bug 1413 - Proposed modification and extensions of TopologyRead module
Proposed modification and extensions of TopologyRead module
Status: NEW
Product: ns-3
Classification: Unclassified
Component: topology-read
ns-3-dev
All All
: P5 normal
Assigned To: Tommaso Pecorella
:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2012-04-18 15:40 EDT by Alex Afanasyev
Modified: 2012-05-06 09:55 EDT (History)
1 user (show)

See Also:


Attachments
Several extensions (3.78 KB, patch)
2012-04-18 15:40 EDT, Alex Afanasyev
Details | Diff
TopologyReader and derivatives are no longer Objects (8.21 KB, patch)
2012-04-18 15:41 EDT, Alex Afanasyev
Details | Diff
Making default constructor for TopologyReader::Link public (1004 bytes, patch)
2012-04-18 15:42 EDT, Alex Afanasyev
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Alex Afanasyev 2012-04-18 15:40:08 EDT
I'm reusing TopologyRead code and I need the following changes.  You may consider to include them to the base version.
Comment 1 Alex Afanasyev 2012-04-18 15:40:42 EDT
Created attachment 1384 [details]
Several extensions
Comment 2 Alex Afanasyev 2012-04-18 15:41:24 EDT
Created attachment 1385 [details]
TopologyReader and derivatives are no longer Objects

To allow smart pointers, TopologyReader is just inherited from SimpleRefCount<TopologyReader> class
Comment 3 Alex Afanasyev 2012-04-18 15:42:23 EDT
Created attachment 1386 [details]
Making default constructor for TopologyReader::Link public

(necessary for use Link in STL containers)
Comment 4 Tommaso Pecorella 2012-05-06 09:55:32 EDT
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.