[Ns-developers] More gratuitous API breakage in ns-3-dev

[Gustavo Carneiro]

2008/7/8 <craigdo at ee.washington.edu>:

> It's not gratuitous.
>
> There were several places in the helpers where we had not completely cut
> over to the final attribute system and they stuck out like sore thumbs.
> Another aspect was having an entirely separate non-attribute-based
> mechanism
> for setting required attributes in an optional way.  This was all discussed
> on the list, I believe, bugs were filed, and there was a "last-call or
> forever hold your peace" to fix it email that came out the morning of the
> day I checked in the change.


This happened after the 3.1 release (5 days after).  I was led to believe
API changes like this were not supposed to happen after 3.1.

>
> That aside, I think that using compiler attributes to flag deprecated API
> is
> a good idea.  Especially for simple changes like the one you mention.  I'm
> not sure about larger changes that could involve multiple instances of
> major
> sections of API.  I'm a little concerned about getting too carried away
> with
> backward compatibility and leaving cruft around, especially at this early
> stage in the life of ns-3 ...


I think it is reasonable that complex API changes to break backward
compatibility in cases when maintaining that compatibility increases
complexity and adds significant maintenance work.

In this case, however, maintaining API is very trivial and does not add any
maintenance burden.  As I show in my patch, just two lines of code can
easily take care of a renamed method and warn the user of the deprecation.


>
> We should probably also go with realtime updates to some form of API
> changes
> readme file as Tom has suggested, so if your code breaks you can
> immediately
> go and see what has happened.


Yes, and the developer that breaks the API should update this file, as
punishment.  That should make developers think twice before deciding to
break the API in the future... ;-)


>
>
> In any case, as we all know, API breakage will happen; but we absolutely
> need to minimize the pain we cause.  I like your idea and hadn't seen it
> before.
>
> -- Craig
>
> > -----Original Message-----
> > From: ns-developers-bounces at ISI.EDU
> > [mailto:ns-developers-bounces at ISI.EDU] On Behalf Of Gustavo Carneiro
> > Sent: Tuesday, July 08, 2008 6:37 AM
> > To: ns-developers
> > Subject: [Ns-developers] More gratuitous API breakage in ns-3-dev
> >
> > I was re-syncing the ns-3-learning-bridge branch with ns-3-dev, and my
> > learning bridge example is not compiling any more:
> >
> > ../examples/csma-bridge.cc: In function 'int main(int, char**)':
> > ../examples/csma-bridge.cc:80: error: 'class ns3::CsmaHelper'
> > has no member
> > named 'SetChannelParameter'
> > ../examples/csma-bridge.cc:81: error: 'class ns3::CsmaHelper'
> > has no member
> > named 'SetChannelParameter'
> >
> >
> > Investigating the matter it is in bug 232.  Just a
> > terminology change, but
> > it so happens that it breaks API!  Come on, is it so hard to provide
> > compatibility for this, something like below.  You could remove the
> > compatibility APIs after a couple of ns-3 releases.  It's
> > very easy if the
> > deprecated APIs are correctly marked like in my patch.
> >
> > This is just gratuitous API breakage and it makes me really
> > sad about NS-3
> > future.  I must express my concern that if following ns-3-dev
> > is made too
> > difficult then people will tend to not use ns-3-dev din their
> > evelopment
> > and, consequently, will be unable to contribute code back to it.
> >
> >
> > diff -r 73e90de6eb47 src/helper/csma-helper.h
> > --- a/src/helper/csma-helper.h    Tue Jul 08 14:18:50 2008 +0100
> > +++ b/src/helper/csma-helper.h    Tue Jul 08 14:31:41 2008 +0100
> > @@ -78,6 +78,17 @@
> >     * by CsmaHelper::Install
> >     */
> >    void SetChannelAttribute (std::string n1, const
> > AttributeValue &v1);
> > +
> > +  /**
> > +   * \param n1 the name of the attribute to set
> > +   * \param v1 the value of the attribute to set
> > +   *
> > +   * Set these attributes on each ns3::CsmaChannel created
> > +   * by CsmaHelper::Install
> > +   *
> > +   * \deprecated renamed to SetChannelAttribute since NS 3.2.
> > +   */
> > +  void SetChannelParameter (std::string n1, const
> > AttributeValue &v1) {
> > SetChannelAttribute (n1, v1); }
> >
> >    /**
> >     * \param filename filename prefix to use for pcap files.
> >
> >
> >
> > --
> > Gustavo J. A. M. Carneiro
> > INESC Porto, Telecommunications and Multimedia Unit
> > "The universe is always one step beyond logic." -- Frank Herbert
> >
>
>
>


-- 
Gustavo J. A. M. Carneiro
INESC Porto, Telecommunications and Multimedia Unit
"The universe is always one step beyond logic." -- Frank Herbert