plumatic / om-tools

Tools for building Om applications
Eclipse Public License 1.0
436 stars 31 forks source link

Any chance of adding multimethod support? #43

Closed krisajenkins closed 10 years ago

krisajenkins commented 10 years ago

Multimethods are made of unicorn dust, and are neat for separating out rendering logic for things like "show some data / show that we're still loading that data / show that data wasn't found". I really like multimethods.

Now that om supports multimethods, is there any chance of you adding defcomponentmethod? (Bonus points if you can think of a less unwieldy name. :-D)

ianconsolata commented 10 years ago

Can you give us an example of what you mean by "om supports mutlimethods", and maybe an example of what it would look like with defcomponent? A quick google search suggests that they still have problems: https://groups.google.com/forum/#!topic/clojurescript/G-ax020u42g

loganlinn commented 10 years ago

@krisajenkins do you have a specific use case in mind for this? I agree this could be useful, but I am weary of introducing this into the API because I've found it's usually sufficient to use cond/get to dispatch component constructor

krisajenkins commented 10 years ago

Thanks for the replies folks.

@jungziege That groups thread ends with the original poster saying multimethods are fixed in 0.7.0. :-)

@loganlinn Imagine this scenario: I've got a system like Facebook that displays different kinds of things in a long list, and I'm using multimethods like so:

(defmulti wall-item-view
  (fn [data] {:type data}))

(defcomponentmethod wall-item-view :post
  ...)

(defcomponentmethod wall-item-view :advert
  ...)

(defcomponentmethod wall-item-view :birthday-message
  ...)

(defcomponent wall-view
  [items owner]
  (render [this]

    (html
     [:div
      [:h1 "Here's Your Wall!"]
      (om/build-all wall-item-view items)])))

What have I gained over just using (cond)? Some pretty cool things.

In short, multimethods are ace. They decouple the dispatch rules from the functions that get called, and so allow you to add actions without touching those rules. And when the number of cases gets large, they're clearer to read too.

Case for the defence rests, m'lud. :-D

krisajenkins commented 10 years ago

I should add that I'm happy to do the work for this code, if you want a PR.

loganlinn commented 10 years ago

Yep, this is a reasonable use case for multimethod. While there are alternate ways to achieve the same dynamic dispatch, this would provide the cleanest interface to accomplish the task.

Side note, I meant to reply yesterday -- must have forgotten to hit submit button. As I wanted to think through the implications to adding this to the API, I have some work in progress that is almost complete. Will hopefully have a PR today.

Thanks!

krisajenkins commented 10 years ago

Woo, thanks! That's great news (and will really help clean up my current project!)

(FWIW, my delay in responding initially was because it took me three drafts to make the case for multimethod support clearly. :-))