[Ns-developers] Learning Bridge has been merged

[Gustavo Carneiro]

2008/7/21 Mathieu Lacage <mathieu.lacage at sophia.inria.fr>:

>
> On Mon, 2008-07-21 at 18:17 +0100, Gustavo Carneiro wrote:
>
> > I just don't understand what other reason is there for specifically
> > forcing the API break you requested.  I am trying to understand.  Or
> > rather, trying to guess.  I really don't enjoy having to try to guess.
> > I know that in this case the API does not become cleaner with pure
> > virtual methods.  Wondering what else could be the reason...
>
> The first reason is purely syntactical: I would like to try to be
> consistent with what we have done over the past few months and try to
> move towards abstract base classes full of pure virtual methods. In the
> end, it seems that it is a much simpler way to document clearly our APIs
> and it leaves much less room for error or guessing or whatever.


I think it made sense for a while to keep breaking API in order to get an
API as clean as possible, as we were nearing NS-3.1.  Now that we reached
3.1 I think it makes less sense, as stability is more important.


>
>
> In the end, the crux of the matter, though, is that we want to make sure
> that every subclass must implement all methods and the code does not
> compile until you have done it. Yes, I know that this change breaks
> working code but I would argue that this is precisely the goal: make
> sure that every NetDevice implementor gets it right and does not ignore
> this feature to ensure that all NetDevice subclasses work with your
> learning bridge.


Well, I am not sure you will get what you want by making methods pure
virtual.  Sure, developers will probably think about it, just long enough to
find out how to make the code compile again, and then move on.  Most likely,
in a few years the developer that made the netdevice just publishes the code
somewhere and stops maintaining, so it could be just every other user out
there trying to use his code that gets the extra work and frustration.


> I would like to avoid getting into the ns2 position
> where each model would work with only a subset of the other ns2 models
> so, when you wanted to mix and match a couple of them, you would spend
> your time trying to figure out which combinations were possible and
> which were not.


1. optional features are good (modular programming), and 2. you shouldn't
rely on compile-time checking so much, unit tests are the way to go if you
want to really assert things work.


>
>
> You can see a recurring theme: I insisted for adding a BridgeChannel to
> make the BridgeNetDevice behave as much like any other device as
> possible. I think it also makes sense to try to make _every_ subclass as
> consistent and as coherent as possible with the base class feature set:
> avoid conditional features and force every implementor to think about
> it.


I implemented BridgeChannel because it makes perfect sense.


>
>
> To summarize, when an API break is like this, that is, when it triggers
> large compiler errors and is relatively easy to fix, I see no reason to
> not focus on what really matters, that is, ensure consistency when we
> can. If an API break is much more subtle and results in hard to debug
> runtime-only errors, then, I would be much more wary of such a change
> and I would support you in your attempt to obtain API stability. We are
> not there though.


Well, in this case, the errors are in runtime but are not subtle at all:

bool
NetDevice::SupportsPromiscuous () const
{
  return false;
}

void
NetDevice::SetPromiscReceiveCallback (PromiscReceiveCallback cb)
{
  // assert that the virtual method was overridden in a subclass if it
  // claims to support promiscuous mode.
  NS_ASSERT (!SupportsPromiscuous ());
}

bool
NetDevice::SendFrom (Ptr<Packet> packet, const Address& source, const
Address& dest, uint16_t protocolNumber)
{
  NS_FATAL_ERROR ("NetDevice::SendFrom not implemented for " <<
GetInstanceTypeId ().GetName ());
  return false;
}


If a subclass forgets to implement it, you will get a clear error message in
runtime.

But I am tired of this thread.  I'll just make the methods pure virtual,
with an extra commit log note saying whose fault it is for the API break :-)

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