Re: [Gnumed-devel] Gnumed Value Objects dev guide, draft 1

[Karsten Hilbert]

> > > Better to have a single query which grabs a list of object PKs *and* 
> > > fields to instantiate VOs en bloc.
> > Can you expand on that ?
> gmPatient.cPatientSearcher_SQL.get_patient_ids ():
> gets a list of patient IDs (i_id on v_basic_person) Then for each person we 
> are going back and fetching the other fields
> by separate queries. We could do one query (select * from v_basic_person 
> where ....) to do the whole thing at once.
Ah, OK, we already do that such as in
gmClinicalRecord.get_lab_data().

We *don't* do it, however, with patient objects since
instantiating such a beast is a fairly involved, slightly
expensive thing. There are quite a number of places where we
*can* optimize, I am sure. But is that something we'd need to
worry about now as long as whether it's done or not is hidden
from the user ?

For the method in question: the patient searcher passes the
list of ids to the patient pick list dialog (that is invoked
when the search arguments return several patients). We could,
of course, select all fields from v_basic_person right aways
and shove that down pick_list's throat but, hey, we'd loose
the ability to override the picklist by another widget wanting
to display completely different data. Also, since user
interaction is involved, I am not so sure the ultimate jota of
speed matters all to much. Please correct me.

> > > Would it be better to let VO classes have a constructor which takes the 
> > > gmClinicalRecord of the current
> > > patient as a parameter, if only to distribute code around files a bit 
> > > more.
> > How exactly would you like this to work ?
> Hmm, I see now that it gets messy if we confuse the role of the VO 
> constructor between a new
> middleware object and constructing a new backend object as well.
I do think it may make sense to write the likes of
gmPathLab.create_lab_result(pk_patient=None, ...) creators which are
being called from gmClinicalRecord.add_lab_result() with
pk_patient=self.id_patient etc. Does that make sense ?

> > > >         _cmds_store_payload: It's important to perform row locking 
> > > > before writting operation.
> > > Why is this? commits are isolated from each other on postgres anyway.     
> > Uhm, ha ?!? We still need to select for update right before
> > updating to prevent another commit to update our row under the
> > cover. We need to protect against concurrent updates.
> Locking will delay the concurrent update, but it will still happen, just 
> later.
No, it will fail and abort the transaction in case another
update (having locked for update) is going on at the moment.

> With transactions the change occurs now but it's hidden (for the first user) 
> until later.
I think we do need quite a bit more research here. We had a
thread about 2 years back but didn't come to a conclusion,
methinks.

> What's the difference? (except locking slows things down) Is it the auditing 
> mechanism
> that forbids concurrency?
No, nothing to do with that, as far as I know. The audit
triggers automatically have the right atomicity behaviour
given the triggering statements do as they are in the same
transaction.

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