Making Grails controllers more DRY?

Question

I am looking for ways on how to cleanup my Grails controller code. In various controllers i more or less have the same logic..

get the object
check if it exists
etc..

Is there a suggested way on making controller actions reuse common code?

--- solution ---

All answers to the question have contributed to the solution we have implemented.

We created a class that is used in our controllers using the Mixin approach. One of the methods that the mixin exposes is the withObject method. This method takes the domainname from the controller and uses this a base for the method. This behaviour can be overridden of course!

def withObject(object=this.getClass().getName()-"Controller", id="id", Closure c) {
    assert object
    def obj =  grailsApplication.classLoader.loadClass(object).get(params[id])
    if(obj) {
        c.call obj
    } else {
        flash.message = "The object was not found"
        redirect action: "list"
    }
}

So all answers have contributed to the solution! Thanks a lot!

Please edit the question and add an example of your code. At the moment the question is very vague. — ordnungswidrig, Nov 25 '11 at 08:41

score 8 · Accepted Answer · answered Nov 25 '11 at 12:26

I always pull out this blog post when this question comes up:

http://mrpaulwoods.wordpress.com/2011/01/23/a-pattern-to-simplify-grails-controllers/

Basically you have a private helper for various domains in your controllers.

private def withPerson(id="id", Closure c) {
    def person = Person.get(params[id])
    if(person) {
        c.call person
    } else {
        flash.message = "The person was not found."
        redirect action:"list"
    }
}

The way you code the getter is very flexible and a typical use for me (that is not covered in the blog) is for editing etc.

I normally code this way (i like the pattern for its clear division and readability):

 def editIssue() {
    withIssue { Issue issue ->
        def issueTypes = IssueTypeEnum.values().collect {it.text }
        [issueTypes:issueTypes,activePage:"issue", issue: issue]
    }
}

 def doEditIssue(IssueCommand cmd) {
    if(cmd.validate()) {
        withIssue { Issue issue ->
            issue.updateIssue(cmd)
            redirect(action: "show", id: issue.id)
        }
    }
    else {
        def issueTypes = IssueTypeEnum.values().collect {it.text }
        render(view: "edit", model:[issueTypes:issueTypes,issue:cmd,activePage:"issue"])
    }
}

With my getter helper being:

private def withIssue( Closure c) {
    def issue = Issue.get(params.id)
    if(issue) {
        c.call issue
    }
    else {
        response.sendError(404)
    }
}

I do think that the mixin method (very similar to the 'extend a common abstract controller' way) is nice too, but this way gives two advantages:

You can type the helper, like you see I do in the closure giving you access to the methods etc in STS/IDEA (not tested Netbeans)
The repetition is not very high, and the ability to change the getter (to use for example BarDomain.findByFoo(params.id) etc)

In the view I bind to edit() I just put an id="${issue.id}" in the <g:form> and it works seamlessly.

Thanks for your comments i more or less combined a few answer in this thread to a working solution at my side. I use the mixin strategy together with a somehow generic withObject method. I will update my question with the code which i am using now. — Marco, Nov 28 '11 at 14:45

Antoine · Answer 2 · 2011-11-25T13:02:20.510

I wouldn't recommend inheritance for that, as you can't spread generic methods in several super classes. Your abstract class would quickly become messy if you have many controllers. You can't use composition (for instance using a Service) because you don't have access to response, render, or params directly from there.

The approach I use is to inject generic methods via Mixins.

@Mixin(ControllerGenericActions)
@Mixin(ControllerUtil)
class BookController {
  def show = &genericShow.curry(Book)

  def exists = {
    render(idExists(Book))
  }
}

The first action show uses a generic method in ControllerGenericActions.groovy, with an argument binded to it. The second use of a mixin idExists method is inside a controller action.

Here is an example code for src/groovy/ControllerGenericActions.groovy

class ControllerGeneric {
  def genericShow(Class clazz) {
    render clazz.get(params.id) as XML
  }
}

and in src/groovy/ControllerUtil.groovy

class ControllerUtil {
  def idExists (Class clazz) {
    return clazz.get(params.id) != null
  }

Not very useful in this case, but you get the idea.

I am not really into the use of closures, so please forgive if i am making a mistake. But if a method is created like 'genericShow(Class clazz)' can you curry that method? I was always in the impression that a method cannot be curried but a closure can. — Marco, Nov 25 '11 at 12:31
That's the reason for using & before the method name: it turns a method into a closure. — Antoine, Nov 25 '11 at 13:01
I've got a question on this solution: Is the ControllerGeneric mixin unit testable? I ask because 'render' is a method provided by Grails while enhancing controllers — Stefan Schubert-Peters, Jan 02 '13 at 12:59

score 0 · Answer 3 · answered Nov 25 '11 at 09:15

0

Implement abstract controller with common methods (use 'protected' directive) and extend from it your real controllers. Do not use 'get' and 'set' words at the beginning of this method's names. Not good, but it works.

answered Nov 25 '11 at 09:15

jenk

1,043
7
8

1

Why do you suggest not use 'get' and 'set' words at the beginning of this method's names? – gotomanners Nov 25 '11 at 09:48
to avoid identifying them as 'getter' or 'setter'. – jenk Nov 25 '11 at 10:58

Making Grails controllers more DRY?

3 Answers3