[jbosscache-dev] Re: JBCACHE-874 (WAS: JBCACHE-315 and 1.4.1)
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 ...