Skip to content

Instantly share code, notes, and snippets.

@ngauthier
Created November 15, 2011 20:44
Show Gist options
  • Star 0 You must be signed in to star a gist
  • Fork 0 You must be signed in to fork a gist
  • Save ngauthier/1368283 to your computer and use it in GitHub Desktop.
Save ngauthier/1368283 to your computer and use it in GitHub Desktop.
Nick's notes on Avdi's "Objects on Rails"

Nick's Notes on Avdi's "Objects on Rails"

Introduction

Do you mean "OO and Procedural programming styles"? Or is Functional what you meant to say. Are they the same? I thought Functional meant like Erlangy w/ immutable variables and recursion and stuff.

4. Yet another frickin' blog app

Here when you define your blog object, you are making the title and subtitle instance methods then instantiating the blog on the root url. Why are these not constant strings if they apply to the "one true blog" this application represents? Or is this all part of the plan so that the app, from the start, does not depend on constants and globals?

EDIT: I see in "6 Submitting posts" that you make it a constant.

5.1 Placeholder blog entries

So you don't TDD controllers? Or does this fall under the "I usually have an acceptance test around this stuff IRL". One thing I've been thinking about recently is more OO controllers, where you could write a spec for the controller object in which you instantiate it and call "index" and you give it a "posts" method and it adds the posts to its internal posts container:

class BlogController
  attr_reader :posts
  attr_reader :blog

  def index
    @posts = []
    @blog = Blog.new
    @posts << @blog.new_entry do |post|
      post.title = "Paint just applied"
      post.body = "Paint just applied. It's a lovely orangey-purple!"
    end
  end
end

describe BlogController do
  before do
    @it = BlogController.new
  end

  it "should have a blog" do
    @it.blog.must_not_be_nil
  end
  it "should have one post" do
    @it.posts.size.must_be 1
  end
  # specs for checking post title and body or something
end

I'm not sure if this is even feasible in rails. But Controllers are one of the things that seem the least like objects (after views) and I want to make them better.

5.3 The Post class

I am warming up to the idea of mock objects in OO code, but I worry about keeping them up to date with the code. What I see happening is that in the Posts spec we have a dependency on the Blog#add\_entry method when describing Post#publish. Then in "5.5 Adding entries to the blog" you have a Blog#add\_entry spec that has nothing to do with Posts. This is really nice that they are decoupled, but doesn't it mean that you could change a Blog's spec and implementation in such a way that Post#publish stops working but all the specs pass? Or, once again, is this where we would have an acceptance test to ensure that this functionality matters?

8 Adding timestamps

The ref:stub link is broken.

9 OMG Dependency Injection!

money shot

12.1 Presenting the Presenter

It may be worth discussing how in other frameworks they call it View and Template, instead of in Rails where we have Presenter and View.

Why in the PresentersHelper spec are you making a new Post? Why not a mock object? I see you use the model's class name farther down, but how about this implementation instead (please excuse my mocking I suck at it):

it "should decorate picture posts with a PicturePostPresenter" do
  # can I just make an object? maybe this is mock() or something?
  post = Object.new
  # mock === which is used in `case` to say this object is a post
  stub(post).===.with(Post){true}
  stub(post).picture?{true}
  @it.present(post, @template).must_be_kind_of(PicturePostPresenter)
end

module PresentersHelper
  def present(model, template)
    case model
    when Post
      if model.picture?
        PicturePostPresenter.new(model, template)
      else
        TextPostPresenter.new(model, template)
      end
    else
      model
    end
  end
end

This also avoids the rails class equality weirdness because you're stubbing the equality operator.

I notice you were a bit unhappy about these class based conditionals. It's these cases where I really wish ruby had type-based overloading of functions or Erlang-esque function definitions based on pattern matching. This would be way easier if we could write:

present(Post) when Post.picture? ->
  PicturePostPresenter(Post)
present(Post) when !Post.picture? ->
  TextPostPresenter(Post)
present(Object) ->
  Object.

This isn't quite erlang because I want parameter pattern matching based on class instead of value. But I really like this style of function definition to behave differently based on parameters.

13 Presenters for REST

In the LinkPresenter#serializable\_hash shouldn't it call super(\*args).merge(links\_hash)?

When you implement first\_before and first\_after how does that actually work? AFAIK so far from reading Post doesn't implement a first class method. Do these methods work? You commented out the rest of the post model but it would be really useful to be able to see the whole thing at this point. It's been a while since the reader has seen a full view of the objects we're working with.

Minor spelling error: templat just before "def url_for" code example.

To make url_for work for an arbitrary object you need to do three things:

  1. extend ActiveModel::Naming
  2. Implement #to_param
  3. Implement the method x_path (i.e. post_path(post)) somehow. Easiest way is via "resources :posts" in the router

Then the template will using Naming to determing its name as a route and look for the route helper. I did in fact try this out in a real rails app like this:

class Blarg
  extend ActiveModel::Naming
  def to_param
    rand(100).to_s # clearly do something better here
  end
end

# in routes:
resources :blarg

# in view
link_to 'blarg', Blarg.new

and it gave /blarg/55 :-)

Unfortunately, in your case of a "Main" blog constant that maps to the root url, a blank or nil to_param doesn't yield /blarg, it raises an error.

I think to truly be restful you'd need to make your blog id=1 and have it be present at /blog/1. Also, it probably makes more sense to monkeypatch polymorphic_url, since that takes a class or instance, and when it's a blog return root_url.

14.2 Adding ActiveRecord

This just occurred to me while reading this section so it's not a well-thought-out idea, but since you mentioned that persistence should be an internal detail of the model, is something like this feasible?

class Post
  # model validations

  def save
    # here we can also customize what parts are persisted if we want
    Persistence.new(serializable_hash).save
  end

  private
  class Persistence < ActiveRecord
  end
end

This means that other objects cannot actually manage a Post's persistence except through the methods Post exposes. So Blog can call Post#save because we implemented it to save via the persistence object.

Just skip the AR validations entirely, let validations be done on the model and in the DB directly (fks and such). If it passes model validations and crashes the db, let it crash because something broke.

This way it's hard to "cheat" from a collaborator, which is usually the biggest source of AR pain, when someone calls a finder directly or saves things when it should not be responsible for that.

An alternative would be to DI the persistence strategy in the constructor. Easy to test, and it also means the back-end would be easier to swap, as long as you play by ActiveModel's rules for persistence strategies (Dirty, Conversion, etc).

EDIT: I see you mention this sort of tactic in 14.7

14.3

Isn't this the point of ActiveModel::Observer? Put the orthogonal code in another model that is notified when Posts are saved (this is how we do push notifications in shortmail, actually).

14.5

typo "you may have notices"

20 Accepting and displaying tags

in Blog::FilteredBlog#entries code where does Taggable come from? If it's Conversions it should be TagList(super) as that is what you called the method. Did you rename it?

EDIT: it looks like you define it in 21 :-)

21 Extracting a Taggable role

in TaggableRecord#save you call super not super(\*args, &block). Is this intentional? Does super pass all params if no params are given?

Having to wrap a Post in Taggable seems pretty gross. Why should the controller need to know it's a taggable object? This is a case where I would prefer the module to be mixed in at all times.

In my opinion, tags should be a collaborator to posts, as models and in the db. Then have a higher level object like TaggedPost for representing a posts and its tags. Then TaggedPosts.with\_tag(tag) would be in charge of finding Posts based on a tagged object.

The controller would save a TaggedPost which would save a post and its associated tags. I think you're letting your serialization persistence strategy drive your implementation, as opposed to the other way around.

Your implementation here makes me think of a half-baked DCI implementation. I do agree that permanently including modules is just shuffling code around and I think it's wrong, but I am not sold on your solution in this case. No hard feelings, bffs4lyfe.

22 Refactoring to a separate ActiveRecord model

Man, I really hate string-based db polymorphism.

So, in this case, if we had gone with my recommendations on 21, we would have to go back and change views, controllers, etc to use a TaggedPost instead of a Post.

However I think if we had used an even earlier suggestion of having the persistence as a class within Post, then all we would have to do is have another persistence class (outside Post) for the Tags, then in the Post model we would have the getters and setters for tags using the relationship and the TagList object. Because the Post model has its persistence in its internal object, it can deal with post attribute persistence and tag persistence within its own persistence strategy. These methods would have to be duplicated to other taggable objects, but the goal would be to make these methods very minimal and specific to the relationship between Post and Tag or SomethingElse and Tag, and not be concerned with the details of persisting the tags themselves.

To use your own words, "I'll write a blog post" :-)

23 Data-Context-Interaction

oh hey, it is DCI. I need to learn more about this, I'll have to get his book.

@steveklabnik
Copy link

Does super pass all params if no params are given?

Yes.

@ngauthier
Copy link
Author

cool, thanks!

@avdi
Copy link

avdi commented Nov 16, 2011

Nick, thanks for taking the time to do this, it's really, really valuable!

@avdi
Copy link

avdi commented Nov 16, 2011

Heh, that "super" question is another of those "I forget not everyone knows that" things. This is why I'm glad I'm getting beta feedback...

@ngauthier
Copy link
Author

ngauthier commented Nov 16, 2011 via email

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment