[IGSTK-Developers] String Events vs. Get Methods

[M. Brian Blake]

Kevin and David,

I understand your points on this one.  The call routine is a bit cumbersome, but in my own programming practices I do similar things to preserve consistency. I just have a few points that may serve as my own clarification.

1. This call routine seems very much in-line with the way other "more substantive" interactions are carried out in the state machine.  Are you guys suggesting that different interaction protocols be adopted by the level of functionality ?   I guess an alternative to this routine would be to "block" and return the string directly. If this is a better routine then how much effort would it be to duplicate this routine in "all" of the code no matter what level of functionality.  No matter how unwieldy the routine I believe, IMHO, that consistency is the most important thing.

2. Also, I believe that consistency will be important if we ever construct an "upper" state machine.  Of course, we could start classifying the state machine interactions by type, but that may make it even more confusing.

Sorry for jumping in, in the middle, just trying to catch up with the conversations as I do my code reviews (smile).

-Brian

M. Brian Blake, Ph.D
Associate Professor
Georgetown University
Washington, DC 20057
mb7 at georgetown.edu
http://www.cs.georgetown.edu/~blakeb/

----- Original Message -----
From: Kevin Gary <kgary at asu.edu>
Date: Thursday, October 6, 2005 4:23 am
Subject: Re: [IGSTK-Developers] String Events vs. Get Methods

> Hey All,
> 
> I have to agree with David here. This seems like an awful
> lot of overkill just to call a Get method. I agree with
> all of David's reasons, and have a few others.
> 
> Effectively the patient name is "returned" to the caller
> via an event. This creates an awkward situation where you
> cannot use a locally scoped variable to store the patient
> name, but instead must provide a class member variable the
> callback can set to store that value. Then when the call
> stack unwinds back to the next line after the original
> RequestPatientName invocation in the app, the app can
> retrieve the patient name from the class member variable.
> To me, it is bad OO practice to introduce class-level
> attributes that are not part of an object's state, but
> are added artifically to support awkward call sequences
> like this.
> 
> What happens when a m_GetPatientnameInfoInput does not
> map to a transition (I only see one transition for it,
> in the image read state). If the SM is in a different
> state, won't this just drop out, never sending an
> event, and leave the original caller (the app) left to
> fend for itself? Would the original app ever know the
> callback was never called and therefore its class member
> variable storing the result is not the result of the
> original request? David's email suggests there is an
> error event in this scenario but I do not see it...
> 
> I would think it would be very hard to convert this type
> of application to be multithreaded if so desired in the
> future.
> 
> David is correct, the code becomes very verbose. Worse, it
> becomes very verbose in the application. I cannot imagine
> folks would be excited to have to write code this way to
> use IGSTK. While I agree that programmer convenience is
> second to program safety, I fail to see how this model is
> safer. In fact, I'd say it is less safe. As the generator
> of an event is decoupled from the target, what happens here
> if there are extra observers registered with the generating
> object. This type of Get method clearly is performing a
> service on behalf of a specific caller.
> 
> I guess I just have a conceptual block on the utility of
> this sequence. I have attached a sequence diagram of the
> invocation sequence, and it just looks awkward to me. A
> Get method means I want to go read/retrieve an attribute
> value of some object, not "I want to go ask an object to
> read its value and tell all observers what that value is".
> 
> Note I do not have an issue with leveraging the state
> machine of DICOMImageReader, as this is appropriate. The
> patient name value should only be available to be read
> if the reader is in a stable state where the value is
> available. I just think we should be able to return that
> value back to the original requestor. While I know this
> may cause an "if" test on the return value, I don't see
> that as less safe than rigging observers and callbacks
> and obfuscating class state variables to achieve the
> equivalent result.
> 
> Finally, adding transitions for Get methods like these
> that are effectively readers and not modifiers of object
> state will inhibit our ability to extend state machines
> to include guards, entry/exit actions, or other mechanisms
> if so desired in the future. A Get method should be
> implemented as an in-state operation (action), or as an
> internal transition (a special type of transition that
> indicates object state will not change and entry/exits/guards
> are not needed - right now we just "const").
> 
> Kevin
> 
> 
> David Gobbi wrote:
> > Hi Everyone,
> > 
> > This came up at the last t-con, and I thought that rather
> > than letting it eat up too much time at the next t-con,
> > it would be better to try to get it at least partially
> > resolved on the mailing list.
> > 
> > Right now, the application get the patient name and other
> > info from the DICOMImageReader by calling a "Request"
> > method, after which the reader will send either a StringEvent
> > that contains the info, or an InvalidRequestEvent.
> > 
> > There has to be application code to do the following:
> > 1) set an observer for DICOMPatientNameEvent, and create a 
> callback 
> > object for the observer
> > 2) set an observer for InvalidRequestEvent, and create a 
> callback object 
> > for the observer
> > 3) define a callback function (called when the request is 
> successful)> 4) define an error function (called when the request 
> failed)> 5) perform the request
> > 
> > I have three issues with this.
> > One is that a large amount of code is required to do a very
> > simple thing, all just to make sure that the patient name is
> > not requested before the file is successfully read.
> > 
> > The second is that the code is separated into three separate
> > chunks (set observers, define callbacks, perform request) which
> > makes it more difficult for a programmer (or reviewer) to look
> > at the code and know whether it is doing the right thing.
> > 
> > Third, if there is a lot of information that is needed from
> > the image (patient, patient ID, physician, hospital, date,
> > default window/level, modality, scan type) then these issues are
> > multiplied by N.
> > 
> > 
> > Personally, I think that the logical place for error checking
> > is at the point in the code where the file is read.  We can
> > trust that the application developer can check this, and if
> > we don't trust the application developer, then we need an
> > application "state machine" that does these checks.
> > 
> > It should be fine if we simply have GetPatientName() etc.
> > methods that return a suitable default (e.g. an empty string)
> > if the file was not successfully read.
> > 
> > - David
> > _______________________________________________
> > IGSTK-Developers mailing list
> > IGSTK-Developers at public.kitware.com
> > http://public.kitware.com/cgi-bin/mailman/listinfo/igstk-developers
> 
> -- 
> ===
> Kevin A. Gary, Ph.D.
> Assistant Professor
> DCST, ASU East
> (480)727-1373
> http://kgary2.east.asu.edu
> kgary at asu.edu
> 
-------------- next part --------------
_______________________________________________
IGSTK-Developers mailing list
IGSTK-Developers at public.kitware.com
http://public.kitware.com/cgi-bin/mailman/listinfo/igstk-developers