David E Jones wrote: > On Jan 30, 2010, at 8:30 PM, Adrian Crum wrote: > >> --- On Sat, 1/30/10, David E Jones <[email protected]> wrote: >>> From: David E Jones <[email protected]> >>> Subject: Re: svn commit: r904921 - in >>> /ofbiz/trunk/framework/base/src/org/ofbiz/base: test/BaseUnitTests.java >>> util/string/UelUtil.java >>> To: [email protected] >>> Date: Saturday, January 30, 2010, 5:37 PM >>> >>> On Jan 30, 2010, at 7:28 PM, Adam Heath wrote: >>>> When I originally came up with the read-only generics >>> for service >>>> engine calls, I tried to make the maps writable. >>> But upon modifying >>>> the entire stack, I discovered it wasn't >>> possible. The service engine >>>> made copies in some cases, so the underlying service >>> implementations >>>> weren't able to send data back at all to the original >>> caller. To >>>> encapsulate this, I made the map read only, by using >>> the ? extends syntax. >>> >>> Maybe this is not the best way to look at it. The point is >>> not that you can't change the context, it is just that any >>> changes to the context are local to the service itself. >>> >>> If I had the service engine to design again I'd consider >>> having it be more like the screen widget with the >>> hierarchical context, and with the context as the local >>> variable space (for all services except those written in >>> plain Java where you can't inject into the variable space on >>> the stack (not that I know of anyway... maybe there is a way >>> and that would be cool). >> When I first got involved with OFBiz, I would write to the service context >> Map because I didn't know any better. Then when I got to know OFBiz better, >> I still wrote to the service context Map because I was lazy. Now I always >> copy the service context Map into a local Map if I want to change things. >> >> The moral of the story is: developers shouldn't be allowed to write to the >> service context Map. If a service needs a Map for local storage, then it >> should create one. > > That's kind of interesting. > > What got me thinking is that the normal practice for Java service is to > create a local variable for pretty much everything in the context anyway (ie > like: String partyId = (String) context.get("partyId");), and there are > things like that all over the place. > > As far as not being to write to the context... why not? Sometimes it's handy > to use the current context as the basis for calling another service. There > are certainly other ways to go about that... but...
I'm not particular tied to either way of doing things. However, I do believe that service implementations should not be allowed to change the original map that the service caller passed in. In that vein, I believe the service engine should be modified to make this copy(or a differential map that only stores the changes). Maybe modify the service model to specify whether the service needs to have a writable copy of the context, or can take an unmodifiable version. > > -David > >
