4
\$\begingroup\$

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

    }



}
\$\endgroup\$

2 Answers 2

3
\$\begingroup\$

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);
    
\$\endgroup\$
1
  • \$\begingroup\$ @sumedha its alright, good job, but as I said, consider testing it, and you will automatically find the leaks \$\endgroup\$ Nov 29, 2015 at 15:01
1
\$\begingroup\$

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.

\$\endgroup\$
2
  • \$\begingroup\$ can you please let me know what do you mean by SVC parameter \$\endgroup\$
    – sumedha
    Nov 30, 2015 at 5:33
  • \$\begingroup\$ 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); } \$\endgroup\$
    – PreFiXAUT
    Nov 30, 2015 at 11:20

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.