Mworks MWorks Core Components → 0.4.4

Comments and changes to this ticket

• 

  Christopher Stawarz May 3rd, 2010 @ 07:29 PM

  • Assigned user set to “Christopher Stawarz”
• 

  David Cox May 4th, 2010 @ 09:51 AM

  One thing that we should be careful to document/keep-straight in the refactored version of the base class is when each of these gets called. This ends up being more important, I think, than what they are supposed to do.

  Basically:

  1. constructor: called when object is created (obviously); other IODevice objects that could conflict with it haven't necessarily been created yet, and channels (which might cause everything to break) haven't been created yet

  2. bool startup(): I think the key here is that this gets called after the channels and all other IODevices have been created (during the "finalize" pass of the parser). This is the device's chance to croak if the requested demands are impossible. Currently, the original IODevice base class splits up this functionality across attachPhysicalDevice, startup, and mapChannelstoCapabilities...
     

  3. bool startDeviceIO(): invoked by an action

  4. bool stopDeviceIO(): ditto

  5. bool shutdown(): Not sure if this is necessary. Maybe keep just for symmetry?

  Another way to approach this might be to ditch the startup and shutdown methods altogether and rely instead on overriding the Component::finalize method instead (which, by definition, gets called at the right time). Already, my IODevice plugins override the Component::addChild method to handle channels (rather than trying to use the convoluted channel classes).
  

  Chris: have you found any specific examples of the startup/shutdown being used in an irreplaceable way?

• 

  Christopher Stawarz May 4th, 2010 @ 03:17 PM

  Chris: have you found any specific examples of the startup/shutdown being used in an irreplaceable way?

  No. It looks like these methods are no-ops for most IODevice subclasses. Also, I'm not sure that shutdown is ever called by the system.

  However, I think I'm in favor of keeping startup (and shutdown just for symmetry), if only because it seems like a more obvious place to put device startup actions than a method called "finalize". Also, in addition to calling startup, IODevice::finalize creates an IODeviceVariableNotification for the device instance. If subclasses were required to override finalize, then they'd have to either call the base class method or create the variable notification themselves. Thus, having subclasses override startup rather than finalize saves subclass implementors a tiny bit of work.

• 

  David Cox May 4th, 2010 @ 03:34 PM

  Works for me. Keeping IODevice::finalize in our territory also gives us wiggle room to add additional/different "finalizing" stuff in the future, while giving users a coherent hook to do post-parse initialization. Actually, having said that, another possibility would be to call it "initialize", thereby avoiding the need for symmetry with "shutdown" (which is sort of redundant with the destructor, in terms of when it would be called, if it were called). Either naming convention is fine by me, though, as long as the call sequence ends up being well documented somewhere for plugin writers.

• 

  Christopher Stawarz May 5th, 2010 @ 12:27 PM

  The more I think about it, the more I like the idea of replacing startup with "initialize" and dropping shutdown. Mostly, I'm imagining how silly the explanation of shutdown would be. ("This method is never called, but maybe you should put something here, just in case.")

• 

  David Cox May 5th, 2010 @ 01:44 PM

  Well, presumably we'd make sure it actually was called if it were included, but giving developers a method that always gets called immediately prior to destruction seems a bit redundant.
  

  I guess one scenario where it could theoretically make sense is if there were some reason an IODevice needed to defer or interrupt its own destruction until later (e.g. "try again in a minute, I'm still powering down the device"). However, I don't know of any real scenario where this is or would be true, so I'm not sure its worth worrying about -- if no one needs it now, then it would be easy to quietly add it in later if it proves to be needed.

• 

  Christopher Stawarz May 5th, 2010 @ 01:56 PM

  • State changed from “open” to “resolved”

  Implemented in commit c2d7e990ffe8827d33fc277dab698867c55547a2