[jbosscache-dev] Re: JBCACHE-874 (WAS: JBCACHE-315 and 1.4.1)

[Manik Surtani]

On 5 Dec 2006, at 19:29, Brian Stansberry wrote:

> Manik Surtani wrote:
>> Brian Stansberry wrote:
>>> It's not a big deal if JBCACHE-874 doesn't go in 1.4.1.CR1, but it
>>> would be nice to get it in so it's out in the wild longer before the
>>> GA.
>>
>> Sorry yes, forgot about this.
>>
>
> If you're delaying 1.4.1.CR1 for this, let me know, because at this
> point I was deferring it a bit pending anything that comes out of this
> thread.

What would you like me to do?  When do you think you could have 874 in?

>
>>>
>>> 1) A bunch of the LinkedList fields in TransactionEntry are
>>> protected. Changing them to LinkedHashSet therefore technically
>>> changes the API. This is an internal class, so I don't think that's
>>> an issue.  Any objections?
>>
>> I'm ok with this, I don't think it will break the
>> OptimisticTransactionEntry subclass.  In fact, is there any
>> reason why this needs to be protected?  Could we make do with
>> stronger access protection than protected?
>>
>
> I don't see any reason why not; this is an internal class. I much  
> prefer
> private fields; hate protected 'cause it's never clear to me if the
> class designer had a strong reason to expose them or just did it "in
> case some subclass wants it some day".  IMHO a protected getter/setter
> with a private field is a much better indication of design intent and
> allows clean subsequent addition of things like lazy initialization.

+1.  And like I said, if we can make do with package-level access for  
the getter/setter and private for the field, I'd go with that if it  
makes sense.

>
>>>
>>> 2) A number of TransactionEntry methods return List and currently
>>> hand out a ref to the internal data structure.  The caller of these
>>> methods logically expects a List so I think the API should remain
>>> the same. So, that implies the methods would do something like
>>> "return new ArrayList(internalSet);"
>>>
>>
>> Hmm, I guess so.  Perhaps in 2.0.0, change this to return a
>> Collection?
>>
>
> Maybe. Depends on whether callers are relying on an expected  
> ordering. I
> had a quick look at these methods yesterday.  At least one is never
> invoked and thus can be dropped. For another, at least in some cases,
> Set semantics are fine, so a possibility for 1.4.1 is an added method
> that exposes the internal Set.

Ok, I'm happy with this.  Like you said, this *is* an internal class  
anyway.

>
> Another possibility for 2.0.0 is to return an Iterator over the  
> internal
> field, which would give the same ordering as a List with no extra
> effort.  I expect all the callers already end up iterating over the  
> List
> anyway.

Is this the case?  What about calling contains() on the List?  I  
thought I saw this somewhere ...