I finally had a chance to look at all the new logic code. First I need
to vent about a couple of things and then I will try to be constructive:
* It took us MONTHS to get the last rewrite of logic to work correctly
with openmrs 1.7.x and the chica code
* We just got it working at the end of October 2011 and Openmrs started
rewriting it again in November 2011.
* The first round of design calls for the logic rewrite happened in
mid-November. I had a two week old baby and did not have time to deal
with it.
* Looking back through dev emails, the logic discussion ended at the end
of November and picked up in mid February
* In mid-February was the first time I saw anything about Calculation.
* On March 5th there was a dev list email from Darius that he and Mike
would be here for one day and wanted to meet with me and some other people
* I was still on part-time maternity leave but rearranged my schedule
with one days notice to meet with them
* When I met with them, I struggled to understand the logic design that
I had not looked at since I had only been back at work for 2 weeks after
maternity leave
* In that meeting, Darius asked us if we could own logic. Basically we
were supposed to own something that got completely redesigned that we
had little or no input into the design that didn't make any sense to us
and stuff that had been fixed repeatedly was broken yet again.
Based on all this, you can see why I might be a little irritated. Yes, I
know you tried to include me in the design but you knew I was pretty
much unavailable. Yes, I could have said no when Darius said we should
own logic but I felt backed into a corner. There was a breakdown in
communication on several fronts. If I would have know we were expected
to own logic, I would have said no to the rewrite to begin with.
Venting done. Now to be constructive:
* The name Calculation is a horrible name for that object. It doesn't
calculate anything! Why not call it "RuleDefinition" since it defines
the properties of the rule and then call CalculationEvaluator "Rule"?
Why was Rule split into two objects anyway? Why not just add a
getParameterDefinitionSet method to the rule interface? Why would you
ever want some object that doesn't evaluate anything being called by
logic? What is the point in that?
* I think the strongly typed results and ParameterDefinitionSets are
nice changes
* Logic cannot hit the database for every token lookup. This is an
unacceptable performance hit for a system like chica. The tokens need to
be cached.
* CalculationEvaluators and Calculations should not be singletons.
Making them singletons is non-thread safe and requires special
programming that is not intuitive to the naive programmer. A new
instance should be created each time.
For a naive programmer, I think the new design complicates things even
more than before and makes it even less accessible for developers to
contribute.
In summary, if the chica group is going to own logic, we would be
willing to own the 0.5 logic that we worked so hard to getting working.
We are not willing to put the time and effort in to get the new
calculation version of logic working.
Thanks,
Tammy
On 3/29/2012 11:29 AM, Michael Seaton wrote:
Hi Tammy,
Is this an issue in the Calculation module? Or is this an issue with
how logic is exposing itself to Calculation? Weren't you involved in
the coding changes that took place during the Sprint? That was meant
to be for you and the Chica team to contribute and own, not for anyone
to impose on you.
It does seem we should cache calculation registrations in memory, we
are not doing this currently. But this shouldn't necessarily be an
issue for Chica, if it is going to the logic_token_registration table
for it's own purposes, should it? I don't really see how exposing
Logic to Calculation is a risk for you, as long as you continue to
access Logic natively as you always have and just provide a couple of
adapter classes to expose it for other via the Calculation module.
Mike
On 03/29/2012 11:05 AM, Tammy Dugan wrote:
There were two major issues that came up when reviewing the changes
with Wyclif,
1. Logic cannot hit the database for every token lookup. This is an
unacceptable performance hit for a system like chica. The tokens need
to be cached.
2. Rules should not be singletons. Making them singletons is
non-thread safe and requires special programming that is not
intuitive to the naive programmer. A new instance should be created
each time.
I also want to point out that we have fixed #2 twice already in
previous versions and now it needs to be fixed for a third time.
Also, #1 was fixed in the previous version and now has to be fixed
again. It gets frustrating that these same problems keeps getting
reintroduced each time logic is refactored and each time I have to
make the case about why it should be that way and we have to fix the
problem. It is important when refactoring to preserve the old behavior!
Because of these issues, we have no immediate plans to update chica
to use the new version of logic with Calculations. It took us months
to get things working before and we just don't have the programming
resources to do it now or in the near future.
Thanks,
Tammy Dugan
On 3/29/2012 10:08 AM, Friedman, Roger (CDC/CGH/DGHA) (CTR) wrote:
It appears that trunk produces 0.5.3 which includes Calculation,
last branch is 0.5.1 which is not mavenized, 0.5.2 is mavenized, can
it be a branch even if not released?
*From:*[email protected] [mailto:[email protected]] *On Behalf Of
*Wyclif Luyima
*Sent:* Friday, March 23, 2012 3:53 PM
*To:* [email protected]
*Subject:* [OPENMRS-DEV] Calculation sprint wrap up
Hi everyone,
For the past 2 weeks, we have been having a sprint on the
calculation module plus making logic and reporting to expose
themselves as calculations. We managed to get all the 27 tickets
specific to the module done by Wednesday. Some work has been done to
retrofit logic and reporting to be exposed as calculations, i went
through the changes in logic with Tammy and Win this afternoon,
Tammy had some interesting points of discussion which i believe she
will send on the dev list.
In general, the code looks pretty good and i think we are ready for
its 1.0 release which could be in the next 1 -2 weeks, apparently we
have to wait for final high level reviews from Darius and Burke as
contributors to its design to confirm if it is actually what they
envisioned.
The sprinter turn up was good which included the core developers,
Mike and Mykola, thank you all for your relentless work.
Have a great weekend.
Wyclif
------------------------------------------------------------------------
Click here to unsubscribe
<mailto:[email protected]?body=SIGNOFF%20openmrs-devel-l>
from OpenMRS Developers' mailing list
------------------------------------------------------------------------
Click here to unsubscribe
<mailto:[email protected]?body=SIGNOFF%20openmrs-devel-l>
from OpenMRS Developers' mailing list
--
Tammy Dugan
CHIRDL Technical Lead
Children's Health Services Research
IU School of Medicine
------------------------------------------------------------------------
Click here to unsubscribe
<mailto:[email protected]?body=SIGNOFF%20openmrs-devel-l>
from OpenMRS Developers' mailing list
------------------------------------------------------------------------
Click here to unsubscribe
<mailto:[email protected]?body=SIGNOFF%20openmrs-devel-l>
from OpenMRS Developers' mailing list
--
Tammy Dugan
CHIRDL Technical Lead
Children's Health Services Research
IU School of Medicine
_________________________________________
To unsubscribe from OpenMRS Developers' mailing list, send an e-mail to
[email protected] with "SIGNOFF openmrs-devel-l" in the body (not
the subject) of your e-mail.
[mailto:[email protected]?body=SIGNOFF%20openmrs-devel-l]
