Batch Processing : Design check with SRP and OCP prinicple

Question

I have to create a batch process which reads data from DB , process it and saves it in db . The batch data is fetched organization wise ( each organization has set of data to be processed) at once.We need to process only specific set of data i.e organization related and save it into db. Hence i have designed below classes . Does this design breaks SRP and OCP?

Should the process method in DepositProcess have the code to iterate the batch data ?

Batch Processor Interface

package com.ibank.batch;
public interface BatchProcessor {
 public void process();
}

Deposit Processort class

package com.ibank.batch;
import java.util.HashMap;
import java.util.Iterator;
import java.util.List;
import java.util.Set;
import com.ibank.batch.svc.DepositBatchProcessorSvc;
public class DepositProcessor implements BatchProcessor {
 @Override
 public void process() {
 DepositBatchProcessorSvc svc = new DepositBatchProcessorSvc();
 HashMap<String, List<String>> batchData = svc.fetchBatchData();
 Set<String> key = batchData.keySet();
 Iterator<String> keyItr = key.iterator();
 while (keyItr.hasNext()) {
 String organizationId = (String) keyItr.next();
 List<String> claimsList = batchData.get(organizationId);
 svc.saveBatchData(claimsList);
 }
 }
}

Service interface and implementation class

package com.ibank.batch.svc;
import java.util.List;
public interface BatchProcessorSvc {
 public Object fetchBatchData();
 public <T> void saveBatchData(List<T> data);
}
package com.ibank.batch.svc;
import java.util.HashMap;
import java.util.List;
public class DepositBatchProcessorSvc implements BatchProcessorSvc {
 @Override
 public HashMap<String, List<String>> fetchBatchData() {
 return null;
 }
 @Override
 public <String> void saveBatchData(List<String> claimList) {
 }
}

DAO interface and implementation

package com.ibank.batch.dao;
import java.util.List;
public interface BatchProcessorDao {
 public Object fetchBatchData();
 public <T> void saveBatchData(List<T> data);
}
package com.ibank.batch.dao;
import java.util.HashMap;
import java.util.List;
public class DepositBatchProcessorDao implements BatchProcessorDao {
 @Override
 public HashMap<String, List<String>> fetchBatchData() {
 return null;
 }
 @Override
 public <String> void saveBatchData(List<String> claimList) {
 }
}

Answer 1

Quick question, have you tried testing your code? Trying doing so, and you will find the leaks in your code.

1. DepositProcessor has a high dependency on DepositBatchProcessorSvc, why not to inject it and make the code flexible (easier to test and change).

   public class DepositProcessor{
    private final DepositBatchProcessorSvc service;
    DepositProcessor(DepositBatchProcessorSvc service){
    this.service = service;
    }
   }
   

2. BatchProcessorSvc could be totally generified,

   /*K type of the object's promary key, and T is the object type*/
   public interface BatchProcessorDao<K,T> {
    @Override
    Map<K, List<T>> fetchBatchData();
   }
   

3. Always try to return interfaces and not implementations, ex:

   HashMap<String, List<String>> fetchBatchData()
   

   could be refactored to

   Map<String, List<String>> fetchBatchData()
   

4. Save should never be fire and forget, I know a lot of db clients do it, but it is really bad, I need to know if things were saved successfully, wishful programming is bad, you could return the objects after saving them

   List<T> saveBatchData(List<T> data);
   

• Sleiman Jneidi: @sumedha its alright, good job, but as I said, consider testing it, and you will automatically find the leaks

Answer 2

Looks like a quite solid code with some good design and space to impelement your own code which I like. I'ld add the @FunctionalInterface to the BatchProcessor, so you can use Lambda-Code (Java8).

But there's one thing I dont really get. Why does the BatchProcessor not take a SVC Parameter instead of using your default, so you can control it. Otherwise that wouldn't make much sence.

Written on my Phone, so excuse Typos please.

• sumedha: can you please let me know what do you mean by SVC parameter
• PreFiXAUT: Sure. That would look something like this: @FunctionalInterface public interface BatchProcessor { // In case you want to have a default-SVC, you can leave this public default void process() { this.process(new DepositBatchProcessorSvc()); } public void procress(BatchProcessorSvc svc); }