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

[craigdo@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.

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.

[ ... ]

> 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.

>   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.

> 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 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).

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.

Regards,

-- Craig