[concurrency-interest] jsr166y.forkjoin API comments

David J. Biesack David.Biesack at sas.com
Thu Jan 24 17:23:05 EST 2008


Been working with the API more and I have a few more comments/suggestions assembled.

Rather than duplicating defaultExecutor() in each Parallel*Array class, provide a single ForkJoinExecutor.Factory static class with a getInstance() method. I like to see the creation API closer to the interface. However, this would require an extra import per use, so I' not pushing it too hard - just seems to be unnecessary duplication.

Even if not, the javadoc in ForkJoinExecutor should indicate how to create and instance and include a {@link ParallelArray#defaultExecutor()}.

The create*() methods should state that the FJE must be non-null.

I find createFromCopy and createWithHandoff method names to be awkward; I have to think through the name each time I use them and tend to want to write .createFromHandoff(). To keep names consistent, I recommend renaming the create*() factor methods as in the following examples

  ParallelDoubleArray pa;
  pa = ParallelDoubleArray.withCopy(myArray, myForkJoinExecutor); // or withCopyOf
  pa = ParallelDoubleArray.withArray(myArray, myForkJoinExecutor);
  pa = ParallelDoubleArray.withSize(n, myForkJoinExecutor);
  pa = ParallelDoubleArray.withCapacity(m, myForkJoinExecutor);

  Thus, code reads more like:
   "pa is a ParallelDoubleArray with a copy of myArray"
   "pa is a ParallelDoubleArray with array myArray"
   "pa is a ParallelDoubleArray with size n"
   "pa is a ParallelDoubleArray with capacity m"

Rename CommonOps.compoundOp as CommonOps.compositeOp as per the well-known Composite design pattern.

Rename the withBounds/withMapping/withFilter methods to simpler bound(...), map(...), and filter(...).

Remove "Indexed" from the withIndexedFilter()/withIndexedMapping() methods; let the overload resolution deal with that. It will make changing code easier - if I decide a filter needs the index, I can simply change its implements clause and implementation and not change all the invocations -- especially true if I defined factory methods to get such filters/mappings.

Having used this a while, I personally don't like the liberal use of "withThis" this and "withThat" throughout the API; it makes it more verbose without really adding much. As per Kent Beck's recommendations (Implementation Patterns, p. 24, Qualified Subclass Name: "Prepend one or more qualifiers to the superclass name to form a subclass name"), rename the resulting ParallelArray*WithBounds, *WithMapping, WithFilter to BoundedParallel*Array, MappedParallel*Array, FilteredParallel*Array. I realize these are not subclasses, but conceptually, they are derivations of Parallel*Array.

  FilteredParallelDoubleArray pa = a.filter(myIndexFilter);

Rename CommonOps.identityPredicate() and nonidentityPredicate() to identicalPredicate() and notIdenticalPredicate();
this seems like the wrong use of "identity" which is normally an op : 

   Object identity(Object x) { return x; }. 

Although System.identityHashCode(Object) is the closest match, but I don't think these methods are implemented in terms of that). In particular, "nonidentity" is not a well known term.

(equalityPredicate/inequalityPredicate are ok)

That's all for today :-)

Thanks

-- 
David J. Biesack     SAS Institute Inc.
(919) 531-7771       SAS Campus Drive
http://www.sas.com   Cary, NC 27513



More information about the Concurrency-interest mailing list