Refactoring switch case statement in different classes

Question

The original code: Refactoring conditional statements among different classes

I am now doing it continually. I fixed a few part as mentioned.

public abstract class BaseService<T> {
 protected Logger logger = Logger.getLogger(this.getClass());
 protected static final String SAVE = "save";
 public abstract Map action(T model) throws Exception;
 }
 public class AcrAccCtrlGateService extends BaseService<AcrAccCtrlGateModel> {
 @Autowired
 private AcrAccCtrlGateDao acrAccCtrlGateDao;
 @Override
 public Map action(AcrAccCtrlGateModel acrAccCtrlGateModel) {
 // TODO Auto-generated method stub
 Map model = new HashMap<String, Object>();
 switch (acrAccCtrlGateModel.getTrStatus()) {
 case CREATE: 
 createTrgStatusAction(acrAccCtrlGateModel);
 model.put(SAVE,acrAccCtrlGateModel);
 break;
 case DELETE: 
 deleteTrgStatusAction(acrAccCtrlGateModel);
 break;
 default : 
 // you can put code here what to do if no one of these cases is correct.
 break;
 }
 return model;
 }
 private void deleteTrgStatusAction(AcrAccCtrlGateModel acrAccCtrlGateModel){
 for(Map<String, String> acrDelList : acrAccCtrlGateModel.getAcrDelList()){
 acrAccCtrlGateDao.delGate(acrDelList);
 acrAccCtrlGateDao.del(acrDelList);
 }
 }
 private void createTrgStatusAction(AcrAccCtrlGateModel acrAccCtrlGateModel) {
 updateModel(acrAccCtrlGateModel);
 saveTheForLoop(acrAccCtrlGateModel); 
 }
 private void updateModel(AcrAccCtrlGateModel acrAccCtrlGateModel){
 acrAccCtrlGateDao.saveGate(acrAccCtrlGateModel);
 acrAccCtrlGateModel.setGateId(acrAccCtrlGateModel.getGateId());
 }
 private void saveTheForLoop(AcrAccCtrlGateModel acrAccCtrlGateModel){
 String gateId = acrAccCtrlGateModel.getGateId().toString();
 List<Map<String,String>> acrAreaList = acrAccCtrlGateModel.getAcrAreaList();
 for (Map<String,String> acrArea : acrAreaList) { 
 acrArea.put("gateId", gateId);
 acrArea.put("crerId", acrAccCtrlGateModel.getCrerId());
 acrArea.put("updrId", acrAccCtrlGateModel.getUpdrId());
 acrAccCtrlGateDao.save(acrArea);
 } 
 }
}
public class AcrAccCtrlStdmService extends BaseService<AcrAccCtrlStdmModel> {
 @Autowired
 private AcrAccCtrlStdmDao acrAccCtrlStdmDao;
 @SuppressWarnings({ "rawtypes", "unchecked" })
 public Map action(AcrAccCtrlStdmModel acrAccCtrlStdmModel) throws Exception{
 Map model = new HashMap();
 try{
 switch (acrAccCtrlStdmModel.getTrStatus()) {
 case CREATE: 
 model.put(SAVE,acrAccCtrlStdmDao.add(acrAccCtrlStdmModel));
 break;
 case UPDATE: 
 model.put(SAVE,acrAccCtrlStdmDao.up(acrAccCtrlStdmModel));
 break;
 case DELETE: 
 for(Map<String, String> stdmId : acrAccCtrlStdmModel.getAcrDelList()){
 acrAccCtrlStdmDao.del(stdmId);
 } 
 break;
 default : 
 // you can put code here what to do if no one of these cases is correct.
 break;
 }
 }catch (Exception e) {
 throw new Exception(e);
 }
 return model;
 }
}
public class AcrAccessAreaService extends BaseService<AcrAccessAreaModel> {
 @Autowired
 private AcrAccessAreaDao acrAccessAreaDao;
 @Autowired
 private FileService fileService;
 @SuppressWarnings("rawtypes")
 public Map action(AcrAccessAreaModel acrAccessAreaModel) throws Exception{
 Map model = new HashMap();
 try {
 switch (acrAccessAreaModel.getTrStatus()) {
 case CREATE: 
 createTrgStatusAction(acrAccessAreaModel);
 model.put("accAreaInfo", acrAccessAreaModel);
 break;
 case UPDATE: 
 updateTrgStatusAction(acrAccessAreaModel);
 break;
 case DELETE: 
 deleteTrgStatusAction(acrAccessAreaModel);
 break;
 default : 
 break;
 }
 } catch (Exception e) {
 throw e;
 }
 return model;
 }
 private void deleteTrgStatusAction(AcrAccessAreaModel acrAccessAreaModel) throws Exception {
 // TODO Auto-generated method stub
 for (Map<String,String> accInfo : acrAccessAreaModel.getAccssAreaList()) {
 if(accInfo.get("attrNo") != null && !accInfo.get("attrNo").equals(""))
 {
 FileModel fileModel = new FileModel();
 fileModel.setFileSeq(accInfo.get("accssAreaId").toString());
 fileModel.setAttrNo(Integer.parseInt(accInfo.get("attrNo")));
 fileService.remove(fileModel);
 }
 acrAccessAreaDao.remove(accInfo);
 acrAccessAreaDao.removeAccArea(accInfo);
 acrAccessAreaDao.removeGate(accInfo);
 }
 }
 private void updateTrgStatusAction(AcrAccessAreaModel acrAccessAreaModel) throws Exception {
 // TODO Auto-generated method stub
 if(acrAccessAreaModel.getImgPath()!=null && acrAccessAreaModel.getImgPath().getSize()>0)
 {
 if(acrAccessAreaModel.getAttrNo() != null)
 {
 FileModel fileModel = new FileModel(); 
 fileModel.setFileSeq(acrAccessAreaModel.getAccssAreaId().toString());
 fileModel.setAttrNo(acrAccessAreaModel.getAttrNo());
 fileService.remove(fileModel);
 }
 int attrNo = 0;
 attrNo = fileService.fileWrite(Constants.ServiceId.ACR_COLOR, acrAccessAreaModel.getAccssAreaId().toString(), acrAccessAreaModel.getImgPath());
 if(attrNo != 0)
 acrAccessAreaModel.setAttrNo(attrNo);
 }
 acrAccessAreaDao.modify(acrAccessAreaModel);
 }
 private void createTrgStatusAction(AcrAccessAreaModel acrAccessAreaModel) throws Exception {
 // TODO Auto-generated method stub
 acrAccessAreaDao.add(acrAccessAreaModel);
 acrAccessAreaModel.setTrStatus(TrStatus.UPDATE);
 int attrNo = 0;
 if(acrAccessAreaModel.getImgPath()!=null && acrAccessAreaModel.getImgPath().getSize()>0)
 attrNo = fileService.fileWrite(Constants.ServiceId.ACR_AREA, acrAccessAreaModel.getAccssAreaId().toString(), acrAccessAreaModel.getImgPath());
 if(attrNo != 0)
 {
 acrAccessAreaModel.setAttrNo(attrNo);
 acrAccessAreaDao.modify(acrAccessAreaModel);
 }
 }
}

I feel better about this code and am wondering about some things:

1. Is there any benefit with using generic BaseService<T>?

2. Could I apply strategy, state pattern or polymorphism with this code? If it's possible, could you please give me advice?

Could you look at this as well?

Refactoring duplicated code

Answer 1

Have you heard about the term programming for interface not implementation? If not, then look it up. That is the reason why there should be a abstract class/interface. It makes the things like what I suggested in the linked question possible.

This code can be improved further. Placing another generic type variable U you can place the dao's declaration in the abstract class. It will make it clear that for the particular service what is the model and the DAO at class declaration itself.

public abstract class BaseService<T, U> { 
 protected Logger logger = Logger.getLogger(this.getClass());
 protected static final String SAVE = "save";
 @Autowired
 protected U curDAO;
 public abstract Map action(T model) throws Exception;
}

That is all I can say about this. Hope this helps.

EDIT

Regarding use of generic in the method I am starting to think that in this case it is not a good idea. Use of interface could be better. I have seen generic methods work wonders but that is when the method body is using those generics. Here it is only placing a compile time limitation. If the requirement is to limit the method parameters to a particular type of classes then Interface is the thing to use.

So I will change the code to something like this

public abstract class BaseService { 
 protected Logger logger = Logger.getLogger(this.getClass());
 protected static final String SAVE = "save";
 @Autowired
 protected DAOInterface curDAO;
 public abstract Map action(ModelInterface model) throws Exception;
}