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);
   

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.