This discussion is archived
1 Reply Latest reply: Sep 19, 2012 12:23 PM by jduprez RSS

Guide best implementaion for this scenario.

795729 Newbie
Currently Being Moderated
Hi All,



I'm using below code in 'SelectDAO' class which is implemented using Spring framework.



<java>



private static final String EMP_INFO_PROC = "select_emp_details";

     private static final String DEPT_INFO_PROC     = "select_department_details";



public Object selectPage(Map params,int pageId) throws Exception {



          logger.info("Inside selectPage() pageId:"+pageId);     

          Object returnObject = null;          

          try{               

               String procedureName = "";

               List<ProcedureCallBean> procedureCallBeans = new ArrayList<ProcedureCallBean>();

               switch(pageId)

               {                    

                    case Constants.EMP_DETL:

                    {                                   

                         procedureName = EMP_INFO_PROC;

                         logger.info("Inside selectPage() EMP_INFO_PROCL: ");

                         procedureCallBeans.add(new ProcedureCallBean("p_emp_id","String","IN",ebatsUtil.getParameterValue(params,"p_emp_id")));

                         procedureCallBeans.add(new ProcedureCallBean(RESULT_SET,"","OUT",""));

                         Map<String, Object> empValues = executeProcedure(procedureName,procedureCallBeans);

                         returnObject = processEmpDetl(params,initiativeValues);                         

                         

                         break;

                    }//End of EMP_DETL

                    

                    case Constants.SELECT_DEPT_DETL:

                    {

                         procedureName = DEPT_INFO_PROC;

                         logger.info("Inside selectPage() DEPT_INFO_PROC: ");

                         procedureCallBeans.add(new ProcedureCallBean("p_dept_id","String","IN",ebatsUtil.getParameterValue(params,"p_dept_id")));

                         procedureCallBeans.add(new ProcedureCallBean(RESULT_SET,"","OUT",""));

                         Map<String, Object> deptValues = executeProcedure(procedureName,procedureCallBeans);

                         returnObject = processDeptDetl(params,initiativeValues);                    

                         

                         break;

                    }//End of SELECT_DEPT_DETL

}//End of switch               

               

          }

          catch (EmptyResultDataAccessException e) {

               logger.error("EmptyResultDataAccessException in selectPage: " + e.toString());

               throw e;

          }

          catch (DataAccessException ex) {

               ex.printStackTrace();

logger.error("DataAccessException in selectPage: "+ex.toString());

throw ex;

          }

          catch (Exception e) {

               logger.error("Exception in selectPage: "+e.toString());

               throw e;

          }          

          return returnObject;

</java>



Whenever I have 'select' procedure then I need to add one constant and switch block corresponding to set parameter values for the procedure to execute.



Currently, I have more than 40 select procedure in my project and it is having somany 'switch' cases.



Can someone please let me know how to refractor this code so that we can avoid these switch statements. This DAO used for all the select procedures and it is easy to maintain for readability. I checked Visitor Pattern and Template Pattern but it seems to be we need to write 40 class files for this functionality for implementing 40 procedures.



Please advice how to rewrite this code using design patterns. Please provide some examples too...



Thanks,

Sharath Karnati.
  • 1. Re: Guide best implementation for this scenario.
    jduprez Pro
    Currently Being Moderated
    Hello,

    Let me start with a note on formatting: <java></java> markup doesn't mean anything on this forum. Please use proper {noformat}
    public class MyCode {...}
    {noformat} syntax, which will be renderered like this:
    public class MyCode {...}
    Next, let's talk about your design question:

    First, you may not be using Spring library to its full extent. Last time I used it, Spring included nice XxxTemplateDAO classes, which allowed for handy and concise subclassing. In particular, you may give a more specific return type than Object .

    Second, I have the feeling your DAO classis is very generic (too much?). It's only a bet based on the very partial code you showed, but I suspect your method selectPage returns instances of different classes ( EmployeeInfo or DepartmentInfo ) depending on the pageId. It looks like it would be simpler to have two methods, each one dedicated to only one type of query, and maybe two DAO classes (typically, one EmployeeDAO class meant to create-read-update-delete employee info, and one DepartmentDAO class).

    Third, your method name selectPage looks very suspicious... Are you sure you are implementing only DB access in this DAO? It's only a bet based on the method name, but I suspect you are also putting business logic, or worse, presentation logic, in the same mix.

    Eventually, and probably more to the core of your question:
    Having one extensive switch/case statement is not wrong in itself (well, at least this is one point where all the cases are listed). The problems arise merely if several switch statements exist in multiple parts of the code, switching over the same list of cases.
    There are two well-known approaches to that issue:
    <li>use subclassing: one class per type, all classes implementing a common interface with only as many methods as the number of switches occurrences. Then replace each switch by [getting the instance of the right class + invoke the appropriate method].
    <li>use Java's enum construct with overridden instance methods (this is just a variant of the former approach indeed, with additional type-safety built-in). That may be hardly more tedious though to inject instances of the enum classes via Spring, and to inject dependencies into the enum instances, I don't know if Spring has built-in support for that.

    Best regards,

    J.

    Edited by: jduprez on Sep 19, 2012 9:21 PM - fixed typo in title

    Edited by: jduprez on Sep 19, 2012 9:22 PM
    Pfff, this fixes the post's title, but not the thread title in the topics listing :(

Legend

  • Correct Answers - 10 points
  • Helpful Answers - 5 points