[Ns-developers] A question about downcast netdevice pointer.

[Gustavo Carneiro]

2008/9/29 <craigdo at ee.washington.edu>

>
> > *sigh* why do you always have to write such long replies... :P
>
> As an aside, that particular smiley may be interpreted by (at least)
> Americans in ways you don't intend.


The meaning I intended to convey is that I am being somewhat lazy by not
wanting to read big emails like yours.  So you should take my complaint only
half seriously.


>
>
> Anyway, typing long emails is not my favorite thing to do, especially when
> I
> could be out enjoying myself or doing more productive work.  I write these
> long emails because it is obvious, based on the replies that I get, that I
> am not being understood.  I assume that this is because of communication
> problems.  When this happens, I try to express what I'm talking about from
> different viewpoints using different examples, hoping that I can come up
> with something that will connect.  I have found this helpful in the past.
> It may be the case that you find reading all of this English painful and
> I'm
> actually being counterproductive.  I don't know.  I do know that if I was
> trying to read all of this in Portuguese or French, it would be bad (TM).
> If you have a better way, please, please, let me know.


The problem with long emails is that not many people have time to read
them.  You might not get any objections to your long emails, but it could
some times mean no one had time to read it, not always that they agree with
it.

I suggest you think of email as a presentation, with bullets.  Focus on a
few key ideas, and add just a little text around them, but not too much.


>
>
> [ ... ]
>
> > Here's what I think (and I wish you could be as objective as I try to
> be):
>
> I don't know why you think I'm not being objective.  As I said above, I'm
> trying to communicate what I think is an important point.  I hope you
> realize I am honestly trying to communicate here.  I'm assuming you are a
> good guy who is trying to do his best at this ns-3 stuff as well.
>
> >   1- You point out problems that AggregateObject should be
> > detecting and
> > warning the user, not GetObject.  Swapping dynamic_cast for
> > GetObject would
> > not help in any additional way that proper assertions in
> > AggregateObject
> > can't detect;
>
> This is why I write so many of those words.
>
> I'm *not* trying to draw an operational distinction between how
> dynamic_cast
> and GetObject work in the case you describe.  I never have.  I've already
> said that the results of dynamic_cast and GetObject are identical in the
> simple, constrained case you mention.  I think I've said it several times.
>
> I'm talking about the conceptual model people have when they are using the
> ns-3 Object system and how the APIs hang together.  What I'm pointing out
> is
> that by thinking about the inheritance case of ns-3 Objects as somehow
> different, it leads to thinking about the system in ways that allow other
> errors to creep in and makes clients think about implementation
> dependencies
> for almost no reason.
>
> One of the things I am trying to convey is that it should really, really be
> obvious that:
>
> > > Ptr<Wifi> wifi = CreateObject<Wifi> ();  // 0x123 address
> > > Ptr<Csma> csma = CreateObject<Csma> ();  // 0x456 address
> > > wifi->AggregateObject (csma);
>
> would be a programming error.  This is obvious if you think of the
> situation
> as Objects, but is not so obvious if you think of it as having some Objects
> and a separate dynamic_cast part.  I think the fact that you asked what
> GetObject would do, instead of flatly saying that this is an error
> situation, reinforces my contention that there is an issue in thinking
> about
> dynamic_cast between ns-3 Objects as somehow different and special.
>
> I think that the fact that the system works in the presence of this kind of
> aggregation indicates that there was a similar distinction made during
> implementation, and that this led to the kind of programming error above as
> being legal in ns-3 today.
>
> >   2- I am all in favor of AggregateObject detecting errors;
> > in fact, I was
> > the first to suggest it:
> > http://www.nsnam.org/bugzilla/show_bug.cgi?id=162
>
> We are all in favor of AggregateObject detecting errors, of course.
>
> The issue I am attempting to point out is that if you think of the objects
> in question (CsmaNetDevice and WifiNetDevice) as somehow not possibly being
> part of an aggregation and think about moving between NetDevice and
> CsmaNetDevice through a dynamic_cast, it is not immediately obvious that an
> error even exists.
>
> If you think of the object model in terms of, "[having] GetObject to do a
> downcast is just convenient syntactical sugar" and "we have normal C++
> class
> single inheritance with a very simple dynamic object aggregation facility"
> how can you immediately come to the conclusion that your example has a very
> serious error?  I don't think you can.
>
> That is the core of the point I'm trying to make!  It's got nothing to do
> with taking C++ inheritance away or requiring massive API changes.  It's
> about how looking at the issue slightly differently makes the global system
> a lot easier to understand and explain, and be less error-prone.
>
> >   3- I am not aware of any situation where a downcast via
> > dynamic_cast would
> > not work 100% correctly unless there is already an underlying
> > aggregation
> > error which would/should be detected by AggregateObject instead;
>
> If I decide for some reason to implement a power model for a net device, I
> could implement it using inheritance in my net device or using aggregation
> with my net device.  If I implement this model using inheritance, I could
> use dymanic_cast to get to my power model "interface."  If I use
> aggregation, DynamicCast would not work in this situation where there is no
> error detected by AggregateObject.
>
> GetObject works in both cases.


True, but this does not make a lot of sense to me.  The same person that
implements the power model knows how to access it.  And of course you cannot
change the way something is implemented without changing the way that code
accesses the implementation.


>
>
> >   4- On one hand you say developers should pretend a subclass
> > is completely
> > independent from its base class, like they're two different objects
> > aggregated together.  In that case, how do you explain the
> > code (e.g.) in
> > packet-socket.cc:
>
> No, not at all.  I'm saying that it would be helpful if they used GetObject
> to move between two Objects even if they happen to be implemented as base
> class and subclass.


It might be OK if we don't know any better.  But if you _know_ one is a base
class of the other, I see absolutely nothing wrong with using dynamic cast.


>
>
> > int
> > PacketSocket::Connect(const Address &ad)
> > {
> > [...]
> >   NotifyConnectionSucceeded ();
> >   return 0;
> >  error:
> >   NotifyConnectionFailed ();
> >   return -1;
> > }
> >
> >   Should we be forced to write GetObject<Socket>
> > ()->NotifyConnectionSucceeded () ?
>
> No.  I said you should absolutely not have to do this *very* *explicitly*
> in
> my last email.
>
> There are two points of view here.  Your perspective here seems to be from
> the implementation of the PacketSocket.  The packet socket implementation
> wouldn't have any reason to use GetObject.  It has defined an "interface"
> as
> being Socket + PacketSocket extensions.  It just implements PacketSocket
> using inheritance and virtual methods.  No change.
>
> I'm talking about what happens when someone *uses* a packet socket.
> Presumably they somehow get a pointer to a Socket and want to get at a
> PacketSocket.  A PacketSocket "interface" is *defined* as having all of the
> methods of the Socket plus the additional methods provided by class
> PacketSocket.  You could socket->GetObject<PacketSocket>() and have access
> to all of the "Socket + PacketSocket" methods or you could
> DynamicCast<PacketSocket>(socket) to get access.  What I'm saying is that
> for other reasons not directly related to the specifics of this case, it
> would be preferable to *encourage* people to use the GetObject version.
>
> As I've said several times, I'm not suggesting, and I have never suggested,
> that we must refactor the PacketSocket to exclude the Socket methods and
> make it so the dynamic_cast would not work.  I have not said that we need
> to
> rewrite PacketSocket.  I'm not quite sure how we got here, but here's what
> I
> said last week reiterating the same point:
>
> "What I said was that by using GetObject instead of dynamic_cast, you don't
> have to use different operations to move between objects that were
> aggregated together explicitly, versus objects that were associated
> together
> by a different implementation (inheritance)."
>

If the programmer is well aware of the difference, I don't see why using
different accessors would be bad.  In fact, it might make the code more
readable.  Where I see dynamic_cast being used I know that the pointer is
being tested against a subclass.


>
>
> If you had implemented PacketSocket as two completely separate "interfaces"
> aggregated together you would be *required* to move between them using
> GetObject.  If you had implemented PacketSocket as "PacketSocket inherits
> from Socket," you could use GetObject to move between them OR you could use
> dynamic_cast.
>
> GetObject works in both cases and there's only one consistent easy to
> explain model to consider and remember from the client perspective.
>
> >   5- Finally, you claim there is no downside to using
> > GetObject instead of
> > dynamic cast.  That is debatable for two reasons.  One,
> > because developers
> > already know C++ and know how to use dynamic_cast much better than
> > GetObject, and two because I am not aware of performance
> > comparison between
> > dynamic_cast and GetObject.  GetObject might be much slower.
> > When in doubt
> > I prefer to use dynamic_cast, as it is generated by a
> > compiler with decades
> > of of experience, hence likely to be better optimized.
>
> I claim that thinking about using GetObject is *helpful* for the reasons I
> have outlined; and there is no API change required in doing so.  I claim
> that the presence of ns-3 Objects in a C++ inheritance situation does make
> a
> difference that needs to be considered.  I claim that the current situation
> is hard to define and understand if you are looking at it from a point of
> view that admits aggregation.  I claim that the performance difference in
> implementation between GetObject and DynamicCast could be only a function
> call (i.e. the dynamic_cast happens first in the GetObject and if it
> succeeds that is all that happens).


In case the dynamic cast would fail, maybe the dynamic_cast returns NULL
with much less work than GetObject has to do to iterate over all aggregated
objects, and for each object call GetTypeId(), traverse the typeid
inheritance path towards the root type.  I think you underestimate the
complexity of GetObject.


> I'm not demanding that you, personally, do anything.  If you recall, what I
> wrote was: "If we are going to pronounce a best practice, I think GetObject
> should be that practice [...]."  You can always do whatever you prefer.


I am still not convinced.  The crux of your arguments appears to be that the
programmer is often unsure how types are related, whether by aggregation or
inheritance, or that the code often switches between one or the other style
of relationship.  In that case, always using GetObject makes sense.  But I
don't agree that the programmer does not know exactly what is going on, and
I don't think relationships will change (or at least they shouldn't, API
stability FTW!).  So recommending GetObject as "unconditional best practice"
seems overkill to me.

Maybe the recommented best practice should be more flexible something like
this:

    "DynamicCast works when the programmer has a base type pointer and is
testing against a subclass pointer.  GetObject works when looking for
different objects aggregated, but also works with subclasses, in the same
way as DynamicCast.  If unsure, the programmer should use GetObject, as it
works in all cases."

I just don't want to say "Use GetObject always" as if dynamic cast was
somehow tainted.  dynamic cast _is_ appropriate if you know what you're
doing.  GetObject is probably much slower than dynamic cast; at least that
is what I suspect until proven wrong.  Sorry for being pessimistic.

Thanks,

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