Re: [Gnumed-devel] Comments on 0.2

[Karsten Hilbert]

On Sat, Jun 24, 2006 at 06:57:12PM +1000, Ian Haywood wrote:

> It's not the API but the highly repetitive surrounding boilerplate
> that is required.
I understand.

> I would like to write smart GUI ancestor classes
> so all this error boilerplate can be done once
> for the 90% case where it's the same
That'd be helpful.

> It's possible to write a wrapper function which takes a middleware
> result tuple and results either the real data or an exception (having done 
> the user interaction)
> but this breaks the no-exceptions rule
Don't worry too much about that rule. The real meaning of it
is "always detect errors even if the best you do is aborting
the dang thing".

> and raises the question (if you're going to let me break it)
Sure, I do believe I have said so before. May I suggest we
actively try this approach as soon as 0.2 is out the door ?

> Here's how I would like gmAllergy.create_allergy () to look
> 
> def create_allergy(**kwargs):
> 
>         # sanity checks:
>         # 1) any of the args being None should fail the SQL code
>         # 2) do episode/encounter belong to the same patient ?
> 
>         kwargs['patient_id'] = self.check_encounter_sanity 
> (kwargs['episode_id'], kwargs['encounter_id'])  # if an error, exception goes 
> straight to be caller, we don't need to handle
Yes, in this case the exception can simply propagate upwards
because it is a *programming* error and should really
gracefully abort the program. However, if it was some sort
of user error or benign "error" (trying to add a duplicate
episode, say) it is a bit different: If I would just let the
exception propagate and not handle it properly (somewhere,
anyways) Richard would rightly claim I don't do "basic input
checking".

>         if type(allg_type) != types.IntType:
>               kwargs['allg_type'] = gmPG.run_ro_query ('select id from 
> clin._enum_allergy_type where value=%s', kwargs['allg_type'])[0][0]

>         # This below is now done by a backend trigger
>         ## set patient has_allergy status
>         ## cmd = """delete from clin.allergy_state where fk_patient=%s"""
>         ## queries.append((cmd, [pat_id]))
>         ## cmd = """insert into clin.allergy_state (fk_patient, has_allergy) 
> values (%s, 1)"""
>         ## queries.append((cmd, [pat_id]))
>         # end triggerwork
Nice catch, I will see to it that this trigger is
implemented as soon as possible. Perhaps I can even do it
with a view with no *explicit* "has_allergy" tracking.

>         # again errors go straight to the caller, we don't need to check
>         allergy = cBusinessClass (table='clin.allergy') (kwargs, new=1) # the 
> new flag means create and look in clin.<table>_pk_seq for pk
OK, that's nice. It just barely doesn't cross the line where
I set the rule: an existing business object instance always
stands for an *existing* row in the table (at instantiation,
that is). I really like the fine push towards that line by
your suggestion: allow create but do not come into existance
if creation fails.

> > Sure enough and I really wish people would help find it
> > (such as with the recent "select for update" discussion).
> Well, I'm trying ;-)
I am really thankful for that !

Karsten
-- 
GPG key ID E4071346 @ wwwkeys.pgp.net
E167 67FD A291 2BEA 73BD  4537 78B9 A9F9 E407 1346