Bug 146

Summary: Ipv4Address API cleanup
Product: ns-3 Reporter: Tom Henderson <tomh>
Component: networkAssignee: ns-bugs <ns-bugs>
Status: RESOLVED FIXED    
Severity: normal    
Priority: P3    
Version: pre-release   
Hardware: All   
OS: All   
Attachments: the proposed API changes and source code alignment

Description Tom Henderson 2008-03-17 08:58:50 UTC
This patch proposes to remove the "HostOrder" postfix of the name of getters and setters in Ipv4Address.  Instead, all Getters and Setters are host order; if we introduce network order getters and setters, we could perhaps qualify those with the "NetworkOrder" postfix such as "SetNetworkOrder" (but presently there are no such methods).

It also does a bit of API cleanup/doxygen and removes redundant methods.

(suggested by George and extended by Tom)
Comment 1 Tom Henderson 2008-03-17 08:59:42 UTC
Created attachment 115 [details]
the proposed API changes and source code alignment
Comment 2 Mathieu Lacage 2008-03-17 10:48:09 UTC
(In reply to comment #0)
> This patch proposes to remove the "HostOrder" postfix of the name of getters
> and setters in Ipv4Address.  Instead, all Getters and Setters are host order;
> if we introduce network order getters and setters, we could perhaps qualify
> those with the "NetworkOrder" postfix such as "SetNetworkOrder" (but presently
> there are no such methods).

I think that the long method name is actually nice and descriptive so, I find it weird to make the method names less descriptive.

Comment 3 Tom Henderson 2008-03-17 22:40:27 UTC
(In reply to comment #2)
> (In reply to comment #0)
> > This patch proposes to remove the "HostOrder" postfix of the name of getters
> > and setters in Ipv4Address.  Instead, all Getters and Setters are host order;
> > if we introduce network order getters and setters, we could perhaps qualify
> > those with the "NetworkOrder" postfix such as "SetNetworkOrder" (but presently
> > there are no such methods).
> 
> I think that the long method name is actually nice and descriptive so, I find
> it weird to make the method names less descriptive.
> 

Yes, it is more descriptive, but longer to type, and e.g. the constructor assumes host order without such a qualifier.  All of the getters and setters are host order; we have network byte order methods but they are called Serialize and Deserialize and operate on a byte array.

I could live with GetHostOrder and SetHostOrder if we removed redundant Set() methods already there, and removed the warnings around their usage, but it seems simpler to me to just say Set() and Get().
Comment 4 Mathieu Lacage 2008-03-18 11:13:49 UTC
> > > This patch proposes to remove the "HostOrder" postfix of the name of getters
> > > and setters in Ipv4Address.  Instead, all Getters and Setters are host order;
> > > if we introduce network order getters and setters, we could perhaps qualify
> > > those with the "NetworkOrder" postfix such as "SetNetworkOrder" (but presently
> > > there are no such methods).
> > 
> > I think that the long method name is actually nice and descriptive so, I find
> > it weird to make the method names less descriptive.
> > 
> 
> Yes, it is more descriptive, but longer to type, and e.g. the constructor
> assumes host order without such a qualifier.  All of the getters and setters
> are host order; we have network byte order methods but they are called
> Serialize and Deserialize and operate on a byte array.

ok. That is a good point.
Comment 5 Mathieu Lacage 2008-05-27 18:06:39 UTC
[ping] what is the status of this bug ? Do you still want to apply this ? 
Comment 6 Tom Henderson 2008-05-28 00:12:28 UTC
[pong] I lost track of that one but yes, I still think it is valid.
Comment 7 Mathieu Lacage 2008-05-29 12:34:27 UTC
changeset 1763f7ac8e80