Adrian Crum wrote: > On 5/26/2010 7:01 AM, Adam Heath wrote: >> Adrian Crum wrote: >>> Just a friendly reminder to the OFBiz developers and committers: The >>> screen widget model classes should not have their state altered by >>> renderers. That is not thread-safe and it results in undesirable >>> behavior. >>> >>> I know it's easy to forget - I just deprecated some methods that I wrote >>> where I made that mistake. :-) >>> >>> It might help to think of the widget model classes as being immutable >>> (though technically they aren't). Once they are constructed and put in >>> the cache, they shouldn't be changed. >> >> Then why not be explicit about it? Make the classes final(maybe), >> make their fields final(required), set all such fields only in the >> constructor(required). If construction is more dynamic/complex, use a >> Builder pattern. > > Because there is a difference of opinion about widget model > immutability. I had suggested those changes a few years ago and there > was opposition to it.
What was the opposition? 1: it's stupid 2: a conversation like this: person-a: I think the model should be immutable. You should do it. person-b: I won't do it. person-a: But it's the best thing since sliced bread! person-b: You can't force me to do anything. 3: making the model immutable would remove locking, yes, but then the renderer classes would get more complex, and harder to write/understand. The cost-benefit ratio just doesn't justify it. Having classes immutable means no locking needs to be done to access their data. Immutable in this case means that if they contain references to complex objects, then those objects must also be immutable(collections classes, etc). Also, when immutable classes are used heavily, it tends to lead to algorithms that end up creating local-to-the-method variables to maintain their work state. java1.6 made that *very* fast, by being able to detect this(escape analysis), and such objects no longer get allocated on the heap, but on the stack(which reduces contention, and reduces GC overhead).
