[Ns-developers] Review request of refactoring bug 557

[Tom Henderson]

Faker Moatamri wrote:
> Faker Moatamri wrote:
>> Hi all,
>> Here is a series of patches needed to fix the bug 557. Please apply 
>> the patches in that order on a freshly downloaded version of ns-3-dev.
>> -Add a virtual function NotifyNewAggregate and call it from the 
>> function AggregateObject. The call will iterate on the listed chain to 
>> notify all the objects in the node:
>> http://codereview.appspot.com/65058
>> Result: Compiling and testing works
>> -Moved the internet stack creation to the helper, refactor some 
>> functions and files and replace some others, implement the 
>> NotifyNewAggregate in each of the protocols...:
>>    - Replace SetNscTcp by a more generic function called SetTcp
>>    - The aggregation between two objects is made by the function 
>> NotifyNewAggregate implemented by the protocols icmp, tcp,ipv4,...
>>    - Nsc-tcp-l4 has 2 new functions to set and get the IP stack 
>> implementation/so library.
>>    - Moved test code from tcp-socket-impl and udp-socket-impl files to 
>> tcp-test.cc and udp-test.cc
>> http://codereview.appspot.com/63068
>> Result: Not compiling
>> -Updated the examples to conform with the changes in the internet 
>> stack helper functioning mode and methods. Basically I changed the 
>> calls to work with the refactored API.
>> http://codereview.appspot.com/63069
>> Result: Not compiling
>> -Updated the Python bindings to reflect the changes in the 
>> internet-stack-helper and in the core Object.
>> http://codereview.appspot.com/63070
>> Result: Compiling but testing fails
>> -Updated one python test to make testing works, changed the calls for 
>> internet stack helper.
>> http://codereview.appspot.com/63071
>> Result: Compiling and testing perfectly working :-)
>> Thank you in advance for reviewing my code
>> Best regards
>> Faker Moatamri
>>
>>
> Hello all,
> Thanks Tom for the review, I took into account some of your comments and 
> replied to some of them, I changed the code and you can view the changes 
> in rietveld:
> http://codereview.appspot.com/63068 .
> http://codereview.appspot.com/65058
> And those files remained unchanged:
> http://codereview.appspot.com/63069
> http://codereview.appspot.com/63070
> http://codereview.appspot.com/63071
> Best regards
> Faker Moatamri

Hi, these changes addressed my concerns, thanks.  I will next try to 
merge it with the ns-3-ip-routing repo.

Tom