Wrong level of abstraction

By on Oct 23, 2012 in eng |

Share On GoogleShare On FacebookShare On Twitter

I was refactoring a module in my project and found an interesting case that implementation detail has been exposed through interface. The code before refactoring was actually did its job fine but it just looked strange in the view of abstraction design.
We have a requirement to provide a mechanism to perform basic check on a couple components of our system. The check is just to see if the component is still live and throw exception if the component could not response to the check. We have defined an interface for this functionality.

public interface SystemCheck {
    public void liveCheck() throws SystemCheckException;
}

The module has evolved until I came back later and saw the current interface as shown below.

public interface SystemCheck {
    public void liveCheck(HttpSession session) throws SystemCheckException;
}

At this point you may say “oh, so your system is a web application?” and that is exactly the point here. Interface is uses to define a role of object, to say what the object implementing this interface can do. Since it is abstraction so it is inherently oversimplified. The caller of this interface should know just enough it need to know to call the implementation. If I put HttpSession argument in the interface then I am explicitly say that HTTP is part of this abstraction. Any caller may need to have access to Servlet library to be able to call this interface.

In my case, it turns out that not all implementations need HttpSession. It is just one implementation that need to access configuration values which can be retrieved from ServletContext object. So it is quite strange to make HttpSession as part of the abstraction since I actually don’t want to say that this SystemCheck will always do something related to web system. I just need a couple string configurations from HttpSession so an implementation class could perform its system checking. This HttpSession should be the details of the implementation object not the information in this level of abstraction.

I guess the reason HttpSession was there is somehow related to underlying technical details. The previous version of this module created all SystemCheck implementation in the constructor of Spring MVC controller. My colleague may not familiar with the framework enough to know how to get ServletContext inside the constructor so the controller was made to wait until handler method was called then HttpSession instance was retrieved and passed as a parameter to SystemCheck implementation.

@Controller()
@RequestMapping(value="/system/*")
public class SystemCheckController {
    List systemCheckList;

    public SystemCheckController(){
        systemCheckList = Arrays.asList(new ArchiveCheck(), new MemoryCheck() );
    }

    @RequestMapping(value="check")
    public @ResponseBody String systemCheck(HttpServletRequest req){
        try{
            for(SystemCheck check : systemCheckList){
                check.liveCheck( req.getSession() );
            }

        }catch(SystemCheckException ex){
            return reportFailedSystemCheck(ex);
        }

        return reportSuccessSystemCheck();
    }

    public class ArchiveCheck implements SystemCheck{
        public static final String ARCHIVE_DIR_CONFIG = "ARCHIVE_DIR_CONFIG";

        public void liveCheck(HttpSession session) throws SystemCheckException {
            String dir = session.getServletContext().getInitParameter(ARCHIVE_DIR_CONFIG);
            //perform check
        }
    }
} 

Now if I decide that I don’t want to expose HttpSession in the SystemCheck abstraction I have to find a way to move it out to the constructor of the implementation class.

public interface SystemCheck {
    public void liveCheck() throws SystemCheckException;
}

@Controller()
@RequestMapping(value="/system/*")
public class SystemCheckController {
    List systemCheckList;

    @Autowired
    public SystemCheckController(List systemCheckList){
        this.systemCheckList = systemCheckList;
    }

    @RequestMapping(value="check")
    public @ResponseBody String basicSystemCheck(){
        try{
            for(SystemCheck check : systemCheckList){
                check.liveCheck();
            }
        }catch(SystemCheckException ex){
            return reportFailedSystemCheck(ex);
        }

        return reportSuccessSystemCheck();
    }
}

@Component
public class ArchiveCheck implements SystemCheck{
    public static final String ARCHIVE_DIR_CONFIG = "ARCHIVE_DIR_CONFIG";

    private ServletContext servletContext;

    @Autowired
    public ArchiveCheck(ServletContext servletContext){
        this.servletContext = servletContext;
    }

    public void liveCheck() throws SystemCheckException {
        String dir = servletContext.getInitParameter(ARCHIVE_DIR_CONFIG);
        //perform check
    }
}

Spring 3 internally expose ServletContext as Spring bean so we could request for it in constructor using @Autowired as usual. Now the information related to HTTP is put in the level of implementation class. Our SystemCheck interface goes back to the generic abstraction without the need to expose any too low level implementation details.