[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/1.3.22.1i |
> 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()
encs.sort(by_issue)
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
get_first/last_encounter().
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:
get_first/last_encounter
-> 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.
Karsten
--
GPG key ID E4071346 @ wwwkeys.pgp.net
E167 67FD A291 2BEA 73BD 4537 78B9 A9F9 E407 1346