playrant.md

Have seen a few play apps' source and it feels like massive boilerplate and leaky promise/future/async crud everywhere.

Lets take this:

https://github.com/guardian/frontend/blob/master/article/app/controllers/ArticleController.scala

case class ArticlePage(article: Article, storyPackage: List[Trail], edition: String)

object ArticleController extends Controller with Logging {

  def render(path: String) = Action { implicit request =>
    val promiseOfArticle = Akka.future(lookup(path))
    Async {
      promiseOfArticle.map(_.map { renderArticle }.getOrElse { NotFound })
    }
  }

  private def lookup(path: String)(implicit request: RequestHeader): Option[ArticlePage] = suppressApi404 {
    // .. stuff omitted
  }

  private def renderArticle(model: ArticlePage)(implicit request: RequestHeader): Result = // .. omitted
}

Let's look at that render method. What does it do? It's pretty hard to tell, even with a lot of the code removed. It's trying to render an article. If we just wrote some vanilla code to do that, it would be:

def render(path:String) = {
  renderArticle(lookup(path).getOrElse(NotFound))
}

So what's all that other stuff? We've got all the buzzwords covered:

• promise
• future
• async

Why is this here? Several of this apps controllers have this exact structure. Shouldn't this concern be separated somehow? Even the Action DSL method that starts this off sticks out like a sort thumb. This method is all unnecessary complexity. To figure out what it does, I have to know what futures and promises are, and what Async does, but moreover, almost none of this method's source is specific to the business logic being performed. Why?

Let's see some more boilerplate:

• implicit request - why do I have to type this in every controller? Shouldn't every controller have a request without me typing code?!
• views.html.article - I really have to tell the ArticleController to use HTML and to use the article view?
• Cached - why is this in the controller? This should be a separate concern.

I realize that some of this might be because of Scala's type system, but it just feels like we've taken the same boilerplate from XML or Annotations and made it code and somehow that makes it OK.

I hated repeating myself and telling the framework what it should've already known in XML, XDoclet, and annotations. I don't like it any better in code.

implicit request - Action is just function that takes a request and returns a response. If you don't need to inspect the request you could have as easily used Action ( … ) // No request naming.
views.html.article - Play uses routes that are in scala. If that view doesn't exist it wouldn't compile. There's no convention that controller actions map to a specific named view. Maybe this is a worthwhile improvement. With macro support it would probably be possible to do at compile time w/o reflecting the current object name.
Cached - I'm probably missing the main reason why this is bad.

A nice part of Play is that since everything is so explicit and mostly functional you can compose your common Actions so that you could define a function that goes from a request to an async cached response without so much code.

E.g. instead of

Action { implicit request =>
  Cached(something) {
    Async {
      …dosomething...
    }
  }
}

You could write

MyRequest { implicit request =>

} 

The promise/future fiasco is unfortunate and will be resolved in the next release. It's a result of being bought by typesafe and switching over to their future libraries instead of using a homegrown promise one.

It's also important to notice that the controllers in play are objects, not classes. They're not built up on a per request basis so having a member value that represents a request isn't part of the design.

• Doesn't it seem odd to have public methods in a controller that aren't taking a request and returning a response? Thus, why do I need to put Action on them? They should be assumed to be actions, because what else would they be?
• Lack of view conventions - seems like a mistake in design to me - wouldn't I generally want to render my views based on a sensible convention, especially if it keeps me from having to be explicit and repeating myself? It's like Spring MVC plus some basic safety at compile time.
• Cached - I dunno; caching should be a cross-cutting concern, shouldn't it?

I think explicit is good in some cases, but all this stuff does is obscure the actual code that's specific to the app, and looks more like Spring MVC/Struts than Rails…

I'd rather see more explicit patterns initially since Play was basically re-written from the ground up. Over time there seem to be some absorption occurring in the community and Typesafe/Play are answering.

One nice change which is being rolled out in 2.1 is the ability in a route to have a controller method defined on a class instead of an object only. Previously, with only objects, mocking resources used by controllers could be a real PITA.

As already mentioned the JSON API is being improved along with consolidation of the Promise/Future API's into one Future defined in the Scala 2.10 std libs.

https://github.com/playframework/Play20/wiki/Highlights

Doesn't it seem odd to have public methods in a controller that aren't taking a request and returning a response? Thus, why do I need to put Action on them? They should be assumed to be actions, because what > else would they be?

I think the intent is that controllers are less about the object and more about the functions. The router passes requests to a function that returns a response vs. setting up an object, initializing a bunch of state and then calling one of it's methods. In other words, the object itself is probably the biggest boilerplate. Play would be just as happy if the functions could just exist without having to arbitrarily name spaced, but without the top level object the JVM would be unhappy.

Isn't an object just a container for functions? Why do I need indirection between a URL and "what to do"? I find it very confusing in Java web apps where the urls and controller names/methods don't match up, especially when there is no real reason they should not. a GET to a route like /people should, without explicit configuration, be serviced by, for example, PeopleController.get.

As to "initializing a bunch of state", this can actually make the underlying API more fluent, and git rid of all the implicit cruft. At a simplistic level:

class PeopleController(request:HttpRequest, val response:HttpResponse) {
  def get = { 
    request // available without having to ask for it with code
    response // again, available without having to ask permission in code
  }
}

// framework code:

val controller = PeopleController.new(request,response)
controller.get
doSomethingWith(controller.response)

So, I guess my beef is with API design - I'm in a controller - it's a special type of class/object, so there are huge advantages to making things easier. If the purpose of the class is to service an HTTP request, why not make it as simple as possible for me to access the raw request, headers, params, etc?

Just to give my 2c.

Doesn't it seem odd to have public methods in a controller that aren't taking a request and returning a response?

In Play controllers are just plain old scala objects (potentially built from a bunch of mixins) - extending from play.api.mvc.Controller just gives you a few helpers, other than that, there is no special meaning attached to the Controller type. A controller method is special only in that sense that it needs to produce an Action at some point. This gives the developer full control over how he/she would like to organize their codebase. Personally, I tend to keep only real "Action producer" methods in my controller (just the way you suggested) but it's not enforced.

Lack of view conventions
2) We do have conventions when they make sense but we also try to keep controllers as "framework-free" as possible. This design decision allows developers to add their own conventions very easily, as well as plugin any view technology they prefer

Cached - I dunno; caching should be a cross-cutting concern, shouldn't it?
Indeed. I would have implemented this as function composition

implicit request
This is necessary only if within your action block an underlying method would take it. It's usually a good idea in scala to avoid hiding implicit arguments (i.e. "be explicit with your implicit"). That said, in your example above, I would question if the request needs to be implicit at all or whether passing it twice top-down (once for lookup, once for renderArticle) is the right way to go.

Why is this here? Several of this apps controllers have this exact structure. Shouldn't this concern be separated somehow?
I totally agree with this point. It looks to me the asynchronous rendering+lookup behavior should be somehow encapsulated

Anyway, my main point is that play provides a generic toolkit and we tried to strike a good balance between providing flexibility and avoiding unnecessary sit-ups. I feel like some of your concerns are specific to this codebase (and I actually agree with most of your points) and some of them are related to the fact that it seems that you tend to prefer the implicitness of Rails (which is fine but that's a different strategy than what we try to accomplish with Play).

HTH,
Peter