getoutreach / epf

A framework for keeping your Ember.js apps in sync.
http://epf.io
MIT License
369 stars 33 forks source link

Why is there no `findAll` functionality in EPF? #74

Closed bobjackman closed 10 years ago

bobjackman commented 11 years ago

In ED, I can do something like App.store.find( App.User ) to get all users. However, doing this in EPF doesn't work. Alternatively, App.User.find() fails as well.

I read somewhere that this is considered by EPF authors to be an anti-pattern, but didn't explain why.

Maybe it's just my particular apps that are non-norm, or maybe I'm doing things in a non-best-practice fashion, but I have very frequent occasion to use a findAll-type functionality and in evaluating whether or not EPF is a good switch for us, this strikes me as a large deficiency.

Can you explain why (if intentional) this functionality has been omitted?

sebzincktiz commented 11 years ago

Use session.query(App.User)

bobjackman commented 11 years ago

Thanks, @sebzincktiz. Is this resultset treated as an "all" dataset or as a regular query result (meaning the client has no idea how the query was performed and thus, cannot determine if new models should/shouldn't be included in this set)?

Also, if session.query(model) works, why does model.query() fail?

ghempton commented 11 years ago

@sebzincktiz's solution is currently the correct way to do this. ATM it is not special cased as the "all" result set (meaning you will have to manually add new records). I might add this eventually, but in my experience the naive "all" result set isn't very useful outside of toy applications.

Re: model.query(), I am trying to discourage static methods such as App.Foo.find(). This type of API is not something I am a fan of and also doesn't play well with the injection container.

bobjackman commented 11 years ago

@ghempton, thanks for taking the time to reply. A few items:

ATM it is not special cased as the "all" result set (meaning you will have to manually add new records).

This is a helpful answer to the original question, thank you.

I might add this eventually, but in my experience the naive "all" result set isn't very useful outside of toy applications.

There are a lot of people with existing Ember apps using ED where "all" is a common approach. Maybe I'm being too sensitive here, but it sounds like you just not only called all of us all naive, but also called our apps toys. You may want to reconsider your approach on this one.

I'm part of a team (of about 10 non-naive people) that's building a rather large Ember app (read: not a "toy app"). I'm definitely open to the possibility that there are better options for what we're doing, but in our experience, at least with our particular project, the concept of an "all" dataset has been very useful.

Re: model.query(), I am trying to discourage static methods such as App.Foo.find(). This type of API is not something I am a fan of and also doesn't play well with the injection container.

Interesting. Could you please explain a few things?

ghempton commented 11 years ago

Sorry about calling it naive, we've used all on many apps as well :P. My point here is more that all is just a limited solution to the problem. We found ourselves more often than not using findQuery instead since records are usually filtered, sorted, scoped to the current user, etc. Ideally I would like for it to be possible for all query calls to be automatically updating (and I am not opposed to having all aliased to query with no arguments.

Why are you not a fan of these static methods? Can you elaborate on "doesn't play well with the injection container" (I know what injection is, I just mean, why do you say it doesn't play nice?)

In EPF there is the notion of multiple sessions. When you statically call App.Foo.find() it makes it hard to specify the session you are operating on.

You've stated a couple reasons you don't like it, but haven't provided an alternative. How are you suggesting this be done?

The idea here is to use session.query 'foo' rather than App.Foo.query.

bobjackman commented 11 years ago

@ghempton Thank you for your explanations :)