Spring with Hibernate DAO layer architecture? [closed]

Question

I am using Spring with Hibernate in my web application and I tend to use HibernateDaoSupport because it will open session and close session automatically. I have two entities Route(route_id, source, destination) and Stop(stop_id, latitude, longitude). Relation between these two tables is many-to-many. One route can contain many stops and one stop can contain many routes.

Route.hbm.xml:

<set name="stops" table="route_stop" cascade="all" lazy="false" order-by="stop_id asc">
    <key column="route_id" />
  <many-to-many column="stop_id"  class="com.trackingsystem.model.Stop" />

</set>

Stop.hbm.xml:

<set name="routes" table="route_stop" cascade="all" lazy="false" inverse = "false">
            <key column="stop_id" />
          <many-to-many column="route_id"  class="com.trackingsystem.model.Route" />
        </set>

This is the DAO class:

public class HibernateRouteStopsDAO extends HibernateDaoSupport implements RouteStopsDAO{

    public Set<Stop> getStops(Route route){

        return route.getStops();

    }
    public Route getRoute(int routeId){

        return (Route)getSession().get(Route.class, routeId);

    }
    public Route getRoute(String source, String destination){
        Session session = null;
        Route route = null;
        try{
            session = getSession();
            route = (Route)session.createCriteria(Route.class)
                    .add(Restrictions.eq("source", source))
                    .add(Restrictions.eq("destination", destination)).list().get(0);
        }catch (Exception e) {
            System.out.println("RouteStopsDAO "+e);
        }finally{

        }

        return route;

    }
    @Override
    public void persistRoute(Route route) {
        Session session = null;
        try{
            session = getSession();
            session.save(route);
        }catch (Exception e) {
            System.out.println("RouteStopsDAO "+e);
        }
    }
    @Override
    public void addStops(Route route, Stop stop) {
        Session session = null;

        try{
            session = getSession();
            route.getStops().add(stop);
            stop.getRoutes().add(route);
            session.update(route);
            session.update(stop);
        }catch (Exception e) {
            System.out.println("RouteStopsDAO "+e);
        }finally{
        }
    }
    @Override
    public List<Route> getAllRoutes() {
        // TODO Auto-generated method stub
        Session session = null;
        List<Route> listOfRoutes = new ArrayList<Route>(0);

        try{
            session = getSession();
            listOfRoutes = session.createCriteria(Route.class).list();
            return listOfRoutes;

        }catch (Exception e) {

            System.out.println("RouteStopsDAO "+e);
            return listOfRoutes;
        }finally{

        }
    }
    @Override
    public void updateRoute(Route route) {
        Session session = null;
        try{
            session = getSession();
            session.update(route);
            session.flush();
        }catch (Exception e) {
            System.out.println("RouteStopsDAO "+e);
        }finally{

        }
    }
    @Override
    public void deleteAllStops(Route route) {
        // TODO Auto-generated method stub

        Session session = null;

        try{
            session = getSession();
            session.delete(route);

        }catch (Exception e) {

            System.out.println("RouteStopsDAO "+e);
        }finally{
            //session.close();
        }

    }
    @Override
    public Stop getStop(int stopId) {
        // TODO Auto-generated method stub
        return (Stop) getSession().get(Stop.class, stopId);
    }
    @Override
    public List<Route> getRoutes(String stopName) {
        // TODO Auto-generated method stub
        List<Route> routes = new ArrayList<Route>();
        List<Stop> stops = getSession().createCriteria(Stop.class)
        .add(Restrictions.eq("stopName", stopName)).list();

        for(Stop stop : stops){
            routes.addAll(stop.getRoutes());
        }

        return routes;
    }
    @Override
    public List<Stop> getStops(String stopName) {
        // TODO Auto-generated method stub 
        List<Stop> stops = new ArrayList<Stop>();
        for(Route route : getRoutes(stopName)){
            stops.addAll(route.getStops());
        }
        return stops;
    }
    @Override
    public Stop getStop(String stopName) {
        // TODO Auto-generated method stu
        List<Stop> stops =  getSession().createCriteria(Stop.class)
                .add(Restrictions.eq("stopName", stopName)).list();
        if(stops.size()>0)
            return (Stop)stops.get(0);
        else return null;
    }

}

Here I am not understanding, do I need to start the transaction or HibernateDaoSupport automatically starts one? Please update my DAO class.

Answer 1

I'm not too familiar with HibernateDaoSupport, so just some generic notes about the code:

1. Empty finally blocks are unnecessary:

   }finally{
   
   }
   

2. You should change structures like

   Session session = null;
   
   try {
     session = getSession();
   

   to:

   try {
     final Session session = getSession();
   

   There is no point here to initialize the Session with null.

3. Catching all Exceptions and printing them to the console is not real exception handling. You should handle the errors. (At least show an error message to the user that their transaction was aborted.)

4. The casting here is unnecessary:

   List<Stop> stops = ...
   if(stops.size()>0)
     return (Stop)stops.get(0);
   else return null;
   

   I'd write it as:

   List<Stop> stops = ...
   if (!stops.isEmpty()) {
     return stops.get(0);
   }
   return null;
   

   According to the Code Conventions for the Java Programming Language:

   if statements always use braces {}.

   Omitting them is error-prone.