[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Gnumed-devel] re: commits

From: Karsten Hilbert
Subject: Re: [Gnumed-devel] re: commits
Date: Sat, 18 Dec 2004 16:31:03 +0100
User-agent: Mutt/

> ok, I forgot the issue_id and the episode_id parameters need to  be 
> passed, but the parameters are identical in get_first_encounter and 
> get_last_encounter
> so I think it still can be refactored because the body of the two 
> methods are identical except for the exception message
> and the encounter list index.  That was *a bug* (forgetting to pass the 
> parameters on).
> There was no intended change to what ever occurs in get_encounters( ..), 
> and I don' t think the complete refactoring  would make a difference.
Think again:

def get_encounters()
 - gets the list of all encounters for a patient

def get_first_encounter()
 - gets the earliest encounter for the given issues/episodes

def get_last_encounter()
 - likewise for latest

Suppose some code does the following:

 encs = get_encounters()
 print encs
... many other things ...
 enc = get_first_encounter()

get_first_encounter() will call get_encounters(). get_encounters()
will return the cached encounters. Initially, the cached
encounters were unsorted (sorted by date in your case). They
got sorted by issue outside get_encounters (because they are
passed by reference). If we now rely on the sort order inside
get_encounters we will get the wrong result for

Also, even inside get_encounters one could not simply sort
*once* after retrieving all encounters because the call from
get_first_encounter() to get_encounters() may be constrained
by issue/episode. So at the very minimum get_encounters()
would need to make sure that the *constrained* list is sorted
properly. And this list can change from call to call because
callers can request a *selection* out of the cache by
specifying a range of issues and/or encounters.

I do not think we can refactor the way you did. It *could* be
done this way:

-> calls _get_encounters()
   ->  calls get_encounters()
       and returns sorted results
from which we select [0] or [-1]

Which I am not sure may be what you had intended. I think,
however, the extra method invocation generates more call
overhead than it gains convenience.

GPG key ID E4071346 @
E167 67FD A291 2BEA 73BD  4537 78B9 A9F9 E407 1346

reply via email to

[Prev in Thread] Current Thread [Next in Thread]