Future of findOrCreateEach and dynamic finders

[dmarcelino]

According to PR #843 findOrCreateEach is deprecated:

@tjwebb

Personally I'd like to deprecate (and remove) all dynamic finders, since it's just senseless proliferation of API surface area.

@particlebanana:

I would love to remove the dynamic finders. Whenever we feel like an 0.11 release is ready we can take a pass and remove all that stuff.

The questions are:

• Should we strip the findOrCreateEach from waterline on 0.11?
• Should we strip all dynamic finders code from waterline on 0.11?
• Should we move dynamic finders to another project?
• Any other thoughts regarding dynamic finders?

cc @devinivy, @randallmeeker, @Globegitter, @RWOverdijk

[Globegitter]

The way I see it is that findOrCreateEach indeed should be deprecated and findOrCreate should support both use-cases. Which it is pretty close to supporting right now anyway.

[devinivy]

I don't care if we strip findOrCreateEach or dynamic finders out of the API. But I'm curious how widely the dynamic finders are used. It is a pretty typical feature of ORMs, I think.

If we do remove findOrCreateEach, we should stop using it internally to waterline core. This was brought-up with @Globegitter here.

[devinivy]

Also, familiarize yourself with what findOrCreateEach actually does if you don't already know! It may not be as obvious as one would expect: https://github.com/balderdashy/waterline-adapter-tests/blob/master/interfaces/semantic/findOrCreateEach.test.js#L29

TLDR: 1st argument is an array of attributes to findOne using each of array of objects in 2nd argument.

[Globegitter]

Also on that note, I am halfway-through implementing a findAndModify function with upsert support. See: http://docs.mongodb.org/manual/reference/command/findAndModify/

For that I also followed the current way and implemented a findAndModifyEach, but purely intended as a helper function. I don't want to drift off the main topic of this discussion but I think it fits here and would be a good questions if we want this functionality (I think we should have these slightly more advanced functions. You don't need them very often, but once you do need them they are tedious enough to implement) and how to implement all use-cases without findAndModifyEach.

Also does someone have a list of what all the dynamic finders are?

[dmarcelino]

helper methods:

• createEach
• findOrCreateEach (DEPRECATED)
• findOrCreate
• findOneLike
• findLike
• startsWith
• endsWith
• contains

Based on your Collection attributes you also have dynamic finders. So given a name attribute the following queries will be available:

• findOneByName
• findOneByNameIn
• findOneByNameLike
• findByName
• findByNameIn
• findByNameLike
• countByName
• countByNameIn
• countByNameLike
• nameStartsWith
• nameEndsWith
• nameContains

Taken from https://github.com/balderdashy/waterline#query-methods

[dmarcelino]

You don't need them very often, but once you do need them they are tedious enough to implement) and how to implement all use-cases without findAndModifyEach.

How about moving dynamic finders to a separate (and optional) module/project? That way people who want to use them can use them and waterline core would have a tighter API.

[Globegitter]

I actually just had the same thought - having it as a simple npm i waterline-extended --save without any other setup would be a good solution as well. But how should adapters be handling that then?

[devinivy]

That's a great idea. I do like the simple findBy{attr} and findOneBy{attr}. The rest are unnecessary, but a module that adds them would be a reasonable middle-ground. Especially if a community member who cares about that functionality wanted to lead the project.

[dasher]

Perform a survey by scanning github projects that have waterline as a
dependency and check to see if the dynamic methods or the findAndxx methods
are used - then publish the results.

It's pointless to make a change if those features are used by a majority of
projects.
On 6 Apr 2015 4:45 pm, "Dário" notifications@github.com wrote:

You don't need them very often, but once you do need them they are tedious
enough to implement) and how to implement all use-cases without
findAndModifyEach.

How about moving dynamic finders to a separate (and optional)
module/project? That way people who wanted to use them could use them and
waterline core would have a tighter API.

—
Reply to this email directly or view it on GitHub
https://github.com/balderdashy/waterline/issues/931#issuecomment-90084574
.

[Globegitter]

This should then also be used to simplify things.

There are for example two findOrCreate and two findOrCreateEach functions which makes it a bit complicated to understand what function gets called when and also has more duplicate code. Doing some tests in my app it seems that findOrCreate in lib/waterline/query/composite.js is the 'main' function and gets called all the time, where-as the findOrCreate in lib/waterline/adapter/compoundQueries.js only seems to be called in conjunction with findOrCreateEach (that is when you pass it an array of objects). Then it seems that both findOrCreateEach (in lib/waterline/query/aggregate.js as well as lib/waterline/adapter/aggregateQueries.js) get called all the time.

Anyone here knows more about this behaviour and the reasoning for both functions?

[devinivy]

I don't know if there are any peculiarities around these particular methods. But generally speaking, here's how it works:

Methods in lib/waterline/query live on the Collection object. The Collection object eventually needs to ask an adapter to do its dirty work. The methods in lib/waterline/adapter do the dirty work of delegating specific queries to the relevant connection/adapter.

[Globegitter]

The main reason I am mentioning it is because to begin with it was quite hard to get into what is actually happening since the functions themselves look very similar and well - they look very similar. So there is a lot of almost duplicate code. Similar checks being done, both have call find/findOne and create under circumstances. So I just wonder if there (and possibly also in other functions) is potential to dry up and simplify the code.

I am not sure if one of the two functions could be removed somehow but from the sounds of it you need both. And then I just wonder if they could use both call the same private functions for the overlapping stuff, or maybe only one of the functions actually needs to do the checks, etc.

This is where I am going with it. What are your thoughts?

[tjwebb]

I added a feature that lets you set dynamicFinders: false. Initially, we could just change the default value of this to false for the next minor release, and see how that goes. That'd be less destructive than ripping out a bunch of code.

[Globegitter]

@tjwebb From what I can see this would not disable findOrCreate/findOrCreateEach (nor findAndModify) though.

[anasyb]

I like findAndModify with option flags, its so extendable and battle tested... also the mongo db guys did a good job in documenting many use-cases of it.
I for one:

• don't use dynamic attributes finders, I prefer using consistent code style that always separates Model/action/criteria
• would never use findOrCreateEach, I think its counter intuitive... just like devinivy said in his second comment: its not obvious!! Maybe it has its niche use-cases, not for me though

My two cents :)

[dmarcelino]

Regarding .createEach, @brandonsimpson has commented the following in balderdashy/waterline#980 (comment):

@dmarcelino I think if .create() already accepts arrays and can do multiple inserts then .createEach() is redundant if it's not doing something more such as create each record sequentially. I think adding an options param to .create() would ultimately allow you to drop .createEach() completely.

[Globegitter]

@dmarcelino I agree with the comment - I have also been thinking a bit more about the whole find/findOrCreate as well as update/updateOrCreate (findAndModify as I called it).

Maybe it would make sense to minimize the public API by adding options to the update or find command.

In my opinion both options have pros and cons - less functions means the public API is less cluttered but on the other hand having more options makes each command slightly more complicated.

But either way the code would definitely benefit from cleaning up/refactoring.

[sailsbot]

Thanks for posting, @dmarcelino. I'm a repo bot-- nice to meet you!

It has been 60 days since there have been any updates or new comments on this page. If this issue has been resolved, feel free to disregard the rest of this message. On the other hand, if you are still waiting on a patch, please:

• review our contribution guide to make sure this submission meets our criteria (only verified bugs with documented features, please; no questions, commentary, or bug reports about undocumented features or unofficial plugins)
• create a new issue with the latest information, including updated version details with error messages, failing tests, and a link back to the original issue. This allows GitHub to automatically create a back-reference for future visitors arriving from search engines.

Thanks so much for your help!