Question

This is actually a semi-question, semi-discussion thread. I think a feature is needed in Java that a method(lets say "calcTotalX") can be defined via annotation in a class(i.e. ParallelExecuter) which will executed beforeStart/AfterEnd of another method (lets say doJob) in the same class. So that we can be sure that any class (lets say SortingParallelExecuter ) extending ParallelExecuter and overriding its "doJob" method don't have to know about X, risk forgetting processing X, handle operations events about X, etc.

My Question is that, is there anything in Java that I can do it like that, except AOP. I don't choose AOP because, it makes the code so much distributed and difficult to read. Also the concern here is class/method/attribute specific. So replication of behaviour is not needed for other class/method/attributes.

By the way, if you find this reasonable, please vote for the thread. Thnx

Ok, for being concrete I am adding a sample class which I use for dividing & paralelization.

public abstract class PartitionedParallelExecutor<T> {

private ExecutorService executorService;

private final List<PartitionErrorDesc<T>> errorMap     = new ArrayList<PartitionErrorDesc<T>>();
private final AtomicInteger totalExecutedJobCount      = new AtomicInteger();

private boolean shutdownForced = false;


private final int workerCount;
private final int partitionCount;
protected final List<T> sourceList; 

//Must be implemented via Extender class
protected abstract PartitionErrorDesc<T> doWork(List<T> subList);



public PartitionedParallelExecutor(int workerCount, int partitionCount, List<T> sourceList) {
    super();
    this.workerCount = workerCount;
    this.partitionCount = partitionCount;
    this.sourceList = sourceList;
}



public Object onPerPartitionFail(List<T> subList, PartitionErrorDesc<T> ped){return null;};

public Object onPerPartitionSuccess(List<T> subList){return null;};

public Object onAnyFailDoOnce() {return null;}

public Object onTotalSuccess() {return null;}


public final void  fireAndWait() {

    if(workerCount <= 0 || partitionCount <= 0 ||
            sourceList == null || sourceList.size() == 0){
        throw new IllegalArgumentException();
    }

    ExecutorService executorService = Executors.newFixedThreadPool(workerCount);
    this.executorService = executorService;

    List<List<T>> partitions = partitionList(sourceList, partitionCount);

    for (final List<T> subList : partitions) {

        executorService.execute(new Runnable() {
                                    @Override
                                    public void run() {

                                        PartitionErrorDesc<T> errorDesc = null;

                                        try {
                                            errorDesc = doWork(subList);
                                        } catch (Throwable e) {

                                            errorDesc = new PartitionErrorDesc<T>(subList);
                                            errorDesc.setSuccess(false);
                                            errorDesc.setE(e);
                                            errorDesc.setFailedAtItem(0);
                                        }   

                                        errorMap.add(errorDesc);

                                        if(errorDesc.isSuccess == false) { //failure

                                            onPerPartitionFail(subList, errorDesc);
                                            setShutdownForced(true);

                                            totalExecutedJobCount.addAndGet(errorDesc.getFailedAtItem());
                                            Thread.currentThread().interrupt();
                                            return;
                                        } else { //success
                                            totalExecutedJobCount.addAndGet(subList.size());
                                            onPerPartitionSuccess(subList);
                                        }
                                    }
        });
    }

    executorService.shutdown();

    try {
        executorService.awaitTermination(60, TimeUnit.MINUTES);
    } catch (InterruptedException e) {

        setShutdownForced(true);
        Thread.currentThread().interrupt();
    }

    if (!isShutdownForced()) {
        onTotalSuccess();
    } else {
        onAnyFailDoOnce();
    }
}


private List<List<T>> partitionList(List<T> sourceList , int partitionCount) {
    List<List<T>> partitions = new ArrayList<List<T>>();
    int totalSize = sourceList.size();

    int pageCount = partitionCount;
    int pageSize  = totalSize / pageCount; 
    int remainder = totalSize % (pageSize * pageCount);

    int fromIndex  = 0;
    int toIndex  = 0;
    for(int i = 0;  i < pageCount; i++) {

        fromIndex = toIndex;

        if(toIndex >= totalSize){
            break;
        }

        if ( remainder > i) {
            toIndex = toIndex + pageSize + 1;
        } else {
            toIndex = toIndex + pageSize;
        }

        List<T> subList = sourceList.subList(fromIndex,toIndex);

        partitions.add(subList);
    }

    return partitions;
}

public final void shutdownNow() {
    setShutdownForced(true);
    List<Runnable> runnables = executorService.shutdownNow();

    try {
        if(!executorService.awaitTermination(60,TimeUnit.SECONDS)) {
            LOG.error("pool didnt terminate after 60 seconds in shutdownNow");
        }
    } catch (InterruptedException e) {
        executorService.shutdownNow();
        Thread.currentThread().interrupt();
    }
}

public final boolean isShutdownForced() {
    return shutdownForced;
}

private synchronized void setShutdownForced(boolean shutdownForced) {
    this.shutdownForced = shutdownForced;
}

}

In this example, a programmer who wants to use the class above for doing his work in a multi-threaded way, he must implement "doJob()" but invoke "fireAndWait()". What a more neat way will be, implementing doJob & calling doJob. The rest of things like calculating "totalExecutedJobCount", onPerPartitionFail() must be implemented in AOP manner cross-cut to "doJob" method. Yes, we can implement this functionality in another class and yet any class extending PartitionedParallelExecutor can also extend this AOP behaviour as much as I know. At this point I ask, why these to things (calculating "totalExecutedJobCount", onPerPartitionFail()) must be in another class. They are related with that class, its attributes and method. Looking in an Object-Oriented manner, they must some methods in the same class and called whenever "doJob" ends. That is the question. Hope things are clear now. Thnx for your time.

Was it helpful?

Solution

After our discussion in comments, reading your comment to Peter's answer and inspecting your lately added sample code, I actually understand your question but fail to understand why you see a problem there.

Your method fireAndWait() is actually a Template Method, which is a well-proven OOP design pattern. I think it is totally fine to implement doWork(List<T>) as part of the algorithm, but instruct the user (via JavaDoc) not to call it himself but rely on its being called indirectly. For instance, you often implement Runnable.run() (even in your sample code!), but do not complain that it is not being called by yourself but indirectly via Thread.start() or ExecutorService.execute(). Isn't that the same pattern? Why should one "God method" do everything?

If you do not like your own pattern method approach, feel free to write a PartitionedParallelExecutorStatisticsAspect (sorry for the long name) taking care of this aspect (because that's what it is). Put it into the same package if you like, so it is near the abstract class, and let it do its job. God classes are equally bad as God methods, so arguably using AOP is also a viable alternative. Doing the (parallel partitioned) job is the core concern here, keeping stats is a secondary one. I would be fine with both approaches if implemented cleanly.

Although this topic is partly kinda philosophical and I have seen several close votes here, which is understandable, I am still hoping my comments are helpful. If so, feel free to accept the answer and close the topic so as not to make it an endless discussion thread.

OTHER TIPS

Maybe you should use code like this:

public final void doJob() {
    before();
    inside();
    after();
}

    protected void inside() {
}

private void before() {
}

private void after() {
}

Now you can't overload doJob(), but only inside() method, and you have interceptors: before() and after().

Licensed under: CC-BY-SA with attribution
Not affiliated with StackOverflow
scroll top