On 07/31/2013 10:53 AM, Scott Talbert wrote: > On Tue, 30 Jul 2013, Phil Dibowitz wrote: > >>> Yes, but I think if I added a virtual function at the CRemote level, I >>> would have to add a non-virtual function in all of the CRemote >>> subclasses, not just CRemoteMH. We could do that, but we would just have >>> useless empty implementations in each of the subclasses, so I'm not sure >>> if that's better. >> >> We do this in quite a few places. I think it's cleaner than casting, and also >> more consistent. > > I'll do whatever you want, but I would argue that casting is cleaner. > (I could probably do the casting a little cleaner, by using dynamic_cast > and checking the result.) In this case, we're knowingly performing an > operation that only a subtype of CRemote can perform, so we cast the > CRemote to a CRemoteMH. If we went the other way, we'd be adding dummy > operations to each subclass of CRemote that don't do anything, just > because one subclass can. I think of it like this: just because some cars > have 4 doors, doesn't mean that we have to give all cars 4 doors, but for > "2-door" cars only 2 of the doors work. If I am knowingly dealing with a > 4-door car, I can check whether it is really one, and then attempt to open > the back door, rather than just blindly attempt to open the back door on a > car, where if it is a 2-door, that just won't do anything. :-)
Heh. I like the analogy. But I think the general trend in the code base has
been the CRemote is the superset of all functions that one could _potentially_
do to a remote. If you call an unimplemented one you get LC_UNSUPPORTED.
But my assumption may be poor, lets see.
The only place I see any cast is in libconcord to call ReadRegion on
CRemoteZ_HID.
On the other hand... both CRemoteZ_USBNET and CRemoteZ_HID have a Read()
function which is declared as virtual in remote.h even though they're
"leave-node" functions and shouldn't need to be virtual. Heck we even have:
virtual uint16_t GetWord(uint8_t *x) { return x[1]<<8 | x[0]; };
Man, this needs a cleanup. Ugh.
Anyway, the more canonical examples would be WriteFlash, EraseFlash,
InvalidateFlash, WriteRam, ReadRam, PrepFirmware, etc.
These are all part of the possible functions you could call on a remote. If
you call them on a RemoteZ, they do nothing (one could argue they should
return LC_UNSUPPORTED). You already have to know if a function makes sense on
a given remote, also having to cast it seems silly... the abstract base class
should take care of making that easy for you.
So as far as I can tell, in all places by one, we virtualized, but in one
single example, (ReadRegion), we didn't, so I think we should stick with
consistency.
(It's amazing how much I forget about this codebase when I don't actively hack
on it for a while).
> Yes, I think it is the case where you are updating the "hostname" and
> email address where it takes a long time (~8 seconds!). Perhaps you are
> right - it is trying to contact the website to download something while
> we're waiting around for it to answer us. I've also seen it come back
> with a garbled reply if I come back and try to read the data I just wrote
> too soon. The Link is the most finicky Harmony product I've worked with
> so far. :-)
Ugh. OK, so yeah, should encapsulate that knowledge in comments.
--
Phil Dibowitz p...@ipom.com
Open Source software and tech docs Insanity Palace of Metallica
http://www.phildev.net/ http://www.ipom.com/
"Be who you are and say what you feel, because those who mind don't matter
and those who matter don't mind."
- Dr. Seuss
signature.asc
Description: OpenPGP digital signature
------------------------------------------------------------------------------ Get your SQL database under version control now! Version control is standard for application code, but databases havent caught up. So what steps can you take to put your SQL databases under version control? Why should you start doing it? Read more to find out. http://pubads.g.doubleclick.net/gampad/clk?id=49501711&iu=/4140/ostg.clktrk
_______________________________________________ concordance-devel mailing list concordance-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/concordance-devel
