Repeating code using Grails domain find method

366 views Asked by At

Initial Problem

If you have different methods that basically have only one line different, would there be a way to make it DRY by creating one method.

Example:

def showA( ) {
   def instance

    try {
        instance = A.findById( params.id )
    } catch ( Exception e ) {
        def message = "Error while retrieving details for the given id ${ params.id }, $e"
        log.error message
        responseAsJson( 400, "Invalid id", message )
        return false
    }

    return checkAndRender(instance,  params.id);
}

def showB( ) {

       def instance

        try {
            instance = B.findByBId( params.BId )
        } catch ( Exception e ) {
            def message = "Error while retrieving details for the given id ${ params.id }, $e"
            log.error message
            responseAsJson( 400, "Invalid id", message )
            return false
        }

        return checkAndRender(instance,  params.id);
    }

So, would there be a way to make one method and simply pass as parameter:

  • The domain class
  • the ID to search for

Or would it be better to pass an SQL statement instead?

Update

Based on @dmahapatro comment, I came up with the following:

def showA( ) {
        def clos = {id -> A.findByAId( id ) }
        return findAndShow(clos, params.AId, params )
    }

def showB( ) {
        def clos = {id -> B.findByBId( id ) }
        return findAndShow(clos, params.BId, params )
    }

 def findAndShow(Closure closure, def id, def p)
    {
        def instance
        try {
            instance = closure(id)
        }
        catch ( Exception e ) {
            def message = "Error while retrieving instance details for the given id ${ id }, $e"
            log.error message
            responseAsJson( 400, "Invalid Id", message )
            return false
        }

        return checkAndRender(instance,  id);
    }

Only remaining issues are:

  • How to cleanup even further / make it cleaner.
  • How to bypass warning:

    The [findAndShow] action in [ApiController] accepts a parameter of type [groovy.lang.Closure]. Interface types and abstract class types are not supported as command objects. This parameter will be ignored.

       def findAndShow(Closure closure, def id, def p)
    
1

There are 1 answers

2
agusluc On BEST ANSWER

First thing you should worry if you want a DRY code, is define a better exception handling. Try-catching your code everywhere to handle response to the client is not very DRY, if you put your data-access code in services, you can throw exceptions from them and use a global controller for catch the errors and handle the responses. E.g:

class ErrorController {

    def serverError() {
         if (request.format == 'json') {
            //Code for handling errors in json request, request.exception stores the data about the exception. 
        } else {
            //Code for handling errors in non-json request, e.g:
            render(view: 'error', model: [msg: 'Something went wrong']) //add an error view for this
        }
    }
}

If you like, you can also add handlers for other types of errors (403, 404, etc)

Add to UrlMappings.groovy

    "500"(controller: "error", action: "serverError")

Now you can refactor your code using your new error handling, and reflection:

Controller:

   class MyController {

        def myService

        def show() {
            def result = myService.myFind(params.className,params.id)
            render result as JSON //Render stuff
        }
    }

Service:

       import grails.util.Holders

       class MyService {

            def myFind(String className, Long id) {
                def result = Holders.getGrailsApplication().getDomainClass('com.mypack.'+ className).findById(id)
                if(!result) {
                    throw new ServiceException('really descriptive and usefull error msg')
                }
            }
        }

I defined a ServiceException class so i can add custom logic for it in my ErrorController using the instanceOf operator.