Skip to content
This repository

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP

How should applications be structured (Pt. 2)? #57

Open
zombor opened this Issue June 08, 2012 · 32 comments

3 participants

Jeremy Bush Tom Crayford Gary Bernhardt
Jeremy Bush
zombor commented June 08, 2012

I read through the original discussion of this, and there were lots of good things there. I want to use DCI, and separate my application like Uncle Bob's "lost architecture" talk mentioned (which I've also seen before reading the previous discussion).

I've got an app that looks like this:

module MyApp
  App = Raptor::App.new(self) do
    root :render => "root", :present => :home_page
    path "user" do
      index :to => 'Repository::User.all'
      new
      create :to => 'MyApp::Context::CreateUser.execute', :redirect => :index
    end
  end
end

I've got a context that looks like this:

module MyApp
  module Context
    class CreateUser
      def self.execute(user, params)
        raise Raptor::ValidationError if params['email'].empty? || params['password'].empty?
        user.email = params['email']
        user.password = params['password']
        user.extend(UnregisteredUser).register
        {:user => @user}¬                        
      end

      protected

      module UnregisteredUser
        def register
          Arden::Repository.for(:user).create(self)
        end
      end
    end
  end
end

So questions:

  • Is there a way to get validation errors into the presenter? I'm guessing I'd want to use some kind of DI injector for this, but I'm at a loss on how to write it. It didn't look like Raptor::ValidationError had any way of storing errors or anything like that.
  • I'm also thinking that using Raptor::ValidationError directly inside my context class is a bad idea. Raptor is a delivery mechanism, so I should be making a buffer layer to handle the translation of my app and the delivery mechanism (raptor, in this case)
  • All in all, I think it's a little murky how things get passed around in the system. A little more documentation would help here, I think, explaining specifically what the conventions should be (this seems to be a largely convention based system).

I think the big "how to design an app with raptor" question is how to properly pass data around the layers. DI seems like the preferred way, but it's unclear to me how to write an Injectables class that does this.

Tom Crayford
Collaborator

DCI

I (and I think @garybernhardt) feel that DCI is a big mistake (at least, as I understand it right now). Whilst it's probably better than Rails, including modules into things at runtime makes me want a shower. I don't think we'll ever be doing explicit support for DCI in raptor.

Answers to Questions

Is there a way to get validation errors into the presenter? I'm guessing I'd want to use some kind of DI injector for this, but I'm at a loss on how to write it. It didn't look like Raptor::ValidationError had any way of storing errors or anything like that.

Firstly: Raptor::ValidationError is named badly, it should really be something like Raptor::Routes::ValidationError. It's used for checking that your routes are somewhat sane at startup time, and nothing else. You're not meant to use it externally.

As of right now, there isn't a good way to do this. I think (eventually), we'll keep the injector around when doing internal redirects, and allow you to inject exception into your presenter/views for when you rescue from an exception using raptor. Then your raptor ORM wrapper would store validation errors inside exceptions. This probably comes after doing some more internal design work though (right now the injector gets passed nearly everywhere, and I think we're going to move into it being stored in a thread-local variable).

I'm also thinking that using Raptor::ValidationError directly inside my context class is a bad idea. Raptor is a delivery mechanism, so I should be making a buffer layer to handle the translation of my app and the delivery mechanism (raptor, in this case).

ValidationError is purely meant for route validations, nothing else right now (this conversation is pointing out the naming flaw there rather well).

However, to answer the general point, one of raptor's design goals (for me anyway), is to minimize the amount of translation you have to do between your app and the web (and to push all that translation inside the router). This is pretty ambitious, and I have no idea how well it will pan out.

All in all, I think it's a little murky how things get passed around in the system. A little more documentation would help here, I think, explaining specifically what the conventions should be (this seems to be a largely convention based system).

I think everybody agrees with that. However, raptor isn't baked yet, and we're not that confident on the design (though I'm getting more confident, apart from a couple of (as yet) unsolved issues). The way I'm attempting to increase confidence here is by writing apps in it (which I am doing right now). I think once the design is more baked, we'll be adding significantly more docs.

Jeremy Bush
zombor commented June 08, 2012

injecting modules into things at runtime is all kinds of bad.

I hear you on that. I think doing DCI with decorators is just as fine though. I'm in experimenting stage, and I don't think that using a decorator class as opposed to a pure module is all that much different from a conceptual standpoint. Conceptually, adding behavior to a class is the preferred way, but if that's a large performance hit (as I gathered from the other thread), I don't have a strong resistance to using decorators. It's an implementation detail, and largely irrelevant to this discussion.

Hooking up your application to raptor should be basically the same as if you do pure DCI, or if you use Uncle Bob's Interactors (which I think is just a step below DCI).

I don't think we'll ever be doing explicit support for DCI in raptor.

And I don't think you should be. DCI is an architecture pattern that should be outside of your delivery mechanism chain. There's still a lot of interesting work to be done on how to build web apps with it.

allow you to inject exception into your presenter/views for when you rescue from an exception using raptor. Then your raptor ORM wrapper would store validation errors inside exceptions.

I like this method.

is to minimize the amount of translation you have to do between your app and the web (and to push all that translation inside the router)

Don't you get into Sinatra territory there? That basically sounds like a "route controller" hybrid like sinatra and padrino have. I could be thinking in a different direction though.

The way I'm attempting to increase confidence here is by writing apps in it (which I am doing right now)

Yep, this system is really interesting to me. I don't like how rails does things at all, in how it doesn't really facilitate serving your application. It strongly encourages you to write "rails apps", and I'm seeing that in our application at work.

I'm porting over a php application to ruby for experimentation on "doing it the right way" in a completely separated architecture, and it seems like the way raptor does things can facilitate this easily.

Tom Crayford
Collaborator

Balls, looking through the code suggests that @garybernhardt did mean for users to use ValidationError externally. That's something we've (iirc) never discussed though, and I don't know what his thoughts on it are. Please to be ignoring my failings above.

Don't you get into Sinatra territory there? That basically sounds like a "route controller" hybrid like sinatra and padrino have. I could be thinking in a different direction though.

I probably was quite jumbled when I said that. Raptor (as it stands currently) is all router. There isn't anything else. You never call it, it always calls you (except for ValidationError, but I think that is only semi-baked right now). The router is designed to do all of your HTTP interactions, so you can focus on your application as it stands. We're definitely not looking towards Sinatra territory (Sinatra is very lightweight compared to Raptor, and requires you to explicitly write "controller" actions).

Jeremy Bush
zombor commented June 08, 2012

looking through the code suggests that @garybernhardt did mean for users to use ValidationError externally.

I think that's where I got the idea. :)

Raptor (as it stands currently) is all router. There isn't anything else.

Isn't it basically MVP+router? Seems like the M is the :to part of the route definition (M is your application, basically), and the VP part is hooked up in the route internally as well by convention.

This is why Raptor is very interesting to me, because there is clear separation between the delivery mechanism and your application, and it seems to be encouraged to design your app this way. With rails, sinatra, etc, it's very easy to mix concerns.

To me, MVC and MVP are delivery mechanism patterns, not application design patterns. This is where DCI and Interactors come into play. Kudos on the work done so far.

Gary Bernhardt
Owner

The ValidationError thing was hacked in when I added the seven standard routes. The problem is that the router needs to know when to re-render things. For example, the create action renders new on a ValidationError:

    def create(params={})
      route(:create, "POST", "/",
            {:redirect => :show,
             ValidationError => :new,
             :to => "#{record_module}.create"}.merge(params))
    end

Without a ValidationError exception that Raptor knows about, I don't see a way for the built-in routes to have that behavior. :(

More generally, @zombor, I'm glad that you find the separation between delivery and application so clear! That's exactly what prompted us to start the project: frustration with Rails forcing coupling everywhere.

Any ideas about the validation stuff? ValidationError can definitely go if there's a better way to do it, and especially if that way makes feeding errors to presenters easier.

This kinda comes back to design thread part 1, I guess. It's all about responses from the services: some services need to say "I failed for this reason", and that needs to get to the presenter. We never decided between exceptions and plain old objects for that during the last thread.

Jeremy Bush
zombor commented June 08, 2012

Any ideas about the validation stuff? ValidationError can definitely go if there's a better way to do it, and especially if that way makes feeding errors to presenters easier.

What I do in rails right now is have the context throw an error exception(like Error::NotFound or Error::Validation), then the controller rescues them and displays the correct view with the exception data. The error exceptions contain the error hash, and any other information that is needed to display the error. Maybe somehow work that in like you did ValidationError in the route definition so you could define what kinds of exceptions go where.

I definitely don't think it's a good idea for Raptor to define these exceptions and except the application to use them like I did in my example above.

I think exceptions are clearer to handle, otherwise you end up with hashes/objects that have {:status => :success} in them.

Tom Crayford
Collaborator

You could have ValidationError capable of accepting arbitrary objects, and add it to the injector so that the route we're redirecting to can get at it?

path "articles" do
  update :ValidationError => :edit
  edit
end

The worry here is that you now (probably) want the injector to be capable of injecting a Maybe like thing to the view, so that you can render the :new action without a ValidationException coming in.

We can already do this with optional arguments in the injector (you can inject a method with a default argument, and if there's no injectable available, it'll use the default arg), but we can't do it in the view (at least not via the method_missing thing that's currently proposed). If the view allowed you to inject things via inject(:exception, fallback), this'd probably be fine (as per @garybernhardt's first option in #55). I've written up some other options for fallbacks in #55.

One way around that is to require all validation exceptions go into the presenter at construction time. I'm not sure I like this very much though.

Jeremy Bush
zombor commented June 09, 2012

Basically what I think I want is something that will take the response of the delegator and put it into the presenter somehow. The response would either be an exception, or something that was returned by the delegator call.

Another thing that I thought of was sessions. I assume we should just rack stuff for this. However, since we don't want to be using delivery mechanism stuff in our applications, we'd need some kind of intermediate layer between the app and raptor that handled this stuff. This sounds an awful lot like a controller to me. Maybe I'm thinking about it wrong though? Thoughts?

Tom Crayford
Collaborator

So right now, you can bring in the delegate result by having your initialize method on the presenter take an arg of "subject". Once that pull req is merged, you can ask for "exception"as well in the presenter's constructor.

Gary Bernhardt
Owner

If the presenter can have the exception injected, is it necessary to inject the exception directly into the view with a default? Isn't translating from the injected exception to view-consumable data the job of the presenter?

Jeremy Bush
zombor commented June 09, 2012

I'm of the opinion that the template (what you call the view, i think) should get everything from the presenter (and nowhere else). It provides a clear separation between view logic and display.

This is why I prefer to use mustache for views, but that's neither here nor there.

Gary Bernhardt
Owner

Yep, I agree. Actually, if we're serious about all data in templates coming from presenters, then templates shouldn't have anything directly injected into them at all. Thoughts, @tcrayford?

Tom Crayford
Collaborator

Not directly injecting into templates seems tempting, but I think it messes up with stuff like "I want to show a logout button when the user is logged in". If you want this behaviour in a layout, you have to inject the fact that the user is logged in into every presenter that is on a page that'll show logout, which sucks (and each of those presenters is just going to expose the logged-in-ness to the layout).

Gary Bernhardt
Owner

Well, the classical OO answer to that would be for those presenters to subclass (or mix in—whatever) the logout button presenter. Problem? ;)

Jeremy Bush
zombor commented June 10, 2012

Yep, when using mustache, I have a layout class that contains all this logic, and my pages extend this.

In classical MVC, I'd pass that info in via the controller. Not sure how I'd do it in a system like this. I don't think I understand how Injectors work completely. Can they grab state from other areas of the system (like session, etc)?

Gary Bernhardt
Owner

Yes—an injectable can have stuff injected into it. Your application can define injectables that have lower-level objects injected into them. It's not clear how understandable this will be. Right now it's more confusing than it needs to be because the custom injectable API is ugly.

Tom Crayford
Collaborator

If we have subclassing of presenters, we're still going to end up with a lot of presenters like this:

class ArticlePresenter < UserLoggedInPresenter
  attr_reader :subject
  def initialize(user_logged_in, subject)
    super(user_logged_in)
    @subject = subject
  end
end

You remove a single attr_reader, but still have to pass around the fact that the user is logged in.

Tom Crayford
Collaborator

HERETICAL IDEA

Remove presenters, have views/templates that are all directly mustache (and encourage inheritance of views), screw other templating languages.

Jeremy Bush
zombor commented June 10, 2012

I'm a huge fan of that :)

When initially looking into raptor I learned about Tilt, but was very disappointed that mustache wasn't available. It's basically a really nice presenter pattern library with a strict template language.

Tom Crayford
Collaborator

I actually had a crack at implementing mustache with raptor's current templating design, but threw it out because we couldn't do things like content_for :title. I assume mustache users just override title in the layout subclass for this stuff, right?

Gary Bernhardt
Owner
Jeremy Bush
zombor commented June 10, 2012

Yeah. I don't really like content_for personally. I feel like it's just a hack around the fact there's no real class behind the template.

Gary Bernhardt
Owner
Tom Crayford
Collaborator

I don't think I'm strongly advocating it, just saying we could do it, and it does solve this problem rather neatly.

Jeremy Bush
zombor commented June 10, 2012

I think a lot of the objections are because it doesn't work like erb. You can't use native ruby things in the template like people are used to (for links, etc). I initially had our new project on mustache, but I got overridden and we switched to erb with presenters because of this fact.

Tom Crayford
Collaborator

I don't think (and hope) that my objections with mustache aren't those of the typical rails dev. I dislike a bunch of the design the ruby impl of mustache has. E.g. Why do my presenters inherit from View? There's a very obvious muddling of responsibilities there, the View both renders and presents the data. I think this is due to rubyists disliking introducing new classes (but I'm not sure).

However, we don't need that to be user-visible, we can just pile our own jank on top of mustache to hide our users from it's clunkiness.

Gary Bernhardt
Owner
Jeremy Bush
zombor commented June 10, 2012

On another topic related to some of the the original posts here, I was able to write some basic error validation and get it displaying in the view:

module MyApp
  App = Raptor::App.new(self) do
    root :render => "root", :present => :home_page
    path "user" do
      index :to => 'Repository::User.all'
      new
      create :to => 'MyApp::Context::CreateUser.execute', :redirect => :index, Error::Validation => :new
    end
  end
end
module MyApp
  module Context
    class CreateUser
      def self.execute(user, params)
        if params['email'].empty? || params['password'].empty?
          raise Error::Validation.new(
            {:email => params['email'].empty?, :password => params['password'].empty?}
          )
        end

        user.email = params['email']
        user.password = params['password']
        user.extend(UnregisteredUser).register

        {:status => :success, :user => @user}
      end

      protected

      module UnregisteredUser
        def register
          Arden::Repository.for(:user).create(self)
        end
      end
    end
  end
end

module Error
  class Validation < Exception
    attr_reader :errors

    def initialize(errors)
      @errors = errors
    end
  end
module MyApp
  module Presenters
    class HomePage

    end

    class User
      def initialize(subject, exception = nil)
        @subject = subject
        @exception = exception
      end

      let(:email) { @subject.email }
      let(:email_error) {
        return nil if @exception.nil?
        @exception.errors[:email]
      }

      let(:password_error) {
        return nil if @exception.nil?
        @exception.errors[:password]
      }
    end

    class UserList
      takes :subject
      let(:all) { @subject }
    end
  end
end

This crude implementation seems to work well, and could be expanded to something a lot better without much trouble.

Jeremy Bush
zombor commented June 11, 2012

E.g. Why do my presenters inherit from View? There's a very obvious muddling of responsibilities there, the View both renders and presents the data.

Yeah, that's one nice thing I like about the php version of mustache. You can pass in any kind of presenter class into the constructor and it will use that for it's data. I don't think the ruby version can do that.

Gary Bernhardt
Owner

@tcrayford, when you tried the mustache thing did it actually work at all? Anything we can look at?

Jeremy Bush
zombor commented June 11, 2012

btw, I made repos for my experiment application:

https://github.com/zombor/Raptor-Demo-Delivery
https://github.com/zombor/Raptor-Demo-App

Delivery is the raptor app, and only includes delivery mechanism files.
App is the business logic files of the application.

It currently only does a user registration, and a user logic (stores the user id in the session).

Jeremy Bush

Sometime since I posted this, I wrote a standalone mustache renderer. It lets you use PORO's as your view classes:

https://gist.github.com/zombor/4727040

This eliminates the crappy need to extend and bind your view classes to any kind of renderer, erb, mustache, or anything else. This could probably be adapted easily to use erb as the rendering engine.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Something went wrong with that request. Please try again.