Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve collection lookup #30

Closed

Conversation

ivansglazunov
Copy link

  • mongo test-packages

@dburles
Copy link
Collaborator

dburles commented Mar 10, 2016

Issues #29 and #31

  • Implement ref option
  • No longer look up collections by connection ?
  • Tests

@dburles dburles changed the title #29 Improve collection lookup Mar 10, 2016
@ivansglazunov
Copy link
Author

If null-collection have ref, we can?

new Mongo.Collection(null, { ref: 'test' });
Mongo.Collection.get('test'); // ok
// but
Mongo.Collection.get(null); // still work? or does not add to nullInstances?

@ivansglazunov
Copy link
Author

You can go to my temp c9 to see the process.

@dburles
Copy link
Collaborator

dburles commented Mar 10, 2016

We should keep the same format as before when you call getAll.

@ivansglazunov
Copy link
Author

Oh, my fault.

@ivansglazunov
Copy link
Author

Temp link tests

@ivansglazunov
Copy link
Author

What are we waiting for? The delay on my part?

@dburles
Copy link
Collaborator

dburles commented Mar 14, 2016

Yeah did you want to take care of the above items?

@ivansglazunov
Copy link
Author

Already... ?

@ivansglazunov
Copy link
Author

May be add to collections _ref value? Near _name.

@ivansglazunov
Copy link
Author

This will be determined by what name it is possible refer to the collection if the collection is lost.

@ivansglazunov
Copy link
Author

What else can I do?

@dburles
Copy link
Collaborator

dburles commented Mar 14, 2016

Hey @ivansglazunov would you mind pushing so I can take a look?

@ivansglazunov
Copy link
Author

Of course. I just thought that maybe have not made my part. What about suggestions about _ref?

@dburles
Copy link
Collaborator

dburles commented Mar 14, 2016

I'm not sure why you would need to store it against the collection, what is your thinking on that?

@ivansglazunov
Copy link
Author

shuttler:ref Adding .Ref() helper with link to collection by _name. If it null-collection, _name == undefined. How get collection ref for Mongo.Collection.get from collection?

@ivansglazunov
Copy link
Author

@dburles
Copy link
Collaborator

dburles commented Mar 14, 2016

Hmm.. that seems to be unrelated. What I'm talking about is storing ref on the array of collections in order to use Mongo.Collection.get to look it up

@ivansglazunov
Copy link
Author

Important point:
When you have already received the collection and should be written in a document of this collection ref. Now it is impossible to obtain. For this I propose to add _ref.

@dburles
Copy link
Collaborator

dburles commented Mar 16, 2016

I think we should probably just stick with the performance improvement to the get method. I feel like these changes are too specific to a particular use-case that the majority of people won't ever need, and if that were the case, it's no great hurdle to just make use of collection-extensions and implement it in any way you like.

@ivansglazunov
Copy link
Author

It's a pity. I would like to here was a functional. Universal namespace of collections. At least my team will use this. At a minimum, this functionality does not harm. However, i will have to make his own version of this library, if your version does not have these capabilities.

@ivansglazunov
Copy link
Author

The problem is that it is impossible to implement this functionality in addition to that package in separate package. Either build this package or rebuild to other package.

@dburles
Copy link
Collaborator

dburles commented Mar 16, 2016

The key is that there should be clear benefits to these extra features to anyone who uses the package (beyond just your team), if you can sell me on the practical use-cases it will open up, and why it'll be useful, then I'll reconsider :)

@ivansglazunov
Copy link
Author

It's an easy and beautiful to realize the link between multiple documents in different collections. These collections can be either local or remote. In the application can always be convenient to find on their by ref.It is not a question of opinion or sale. Previously, only the library worked with server collections, and it allows you to work with null-collections. Client collections can be a part of the logic associated overlapping documents in collections.
I remember about all the meteor half of my fellow developers say - it is not necessary for most programmers. Now they are on the meteor. Whom stops the fact that it now no one does not want some change? Better to imagine how it can be useful. And I have the opposite opinion It will need most of the users.

@ivansglazunov
Copy link
Author

And how are you going to look for null collections without ref at all? Then it is better to cut out all the opportunity to look at him. Or add a search through all the options.


Personal...
And i'm afraid to think of what to call my version of Mongo.Collection.get to avoid naming conflicts....

@ivansglazunov
Copy link
Author

It may be confusing your attribute in the collection? So do many of the library, for example collection2.

@ivansglazunov
Copy link
Author

If there is a positive decision, and will be happy to supplement the documentation. :)

@dburles
Copy link
Collaborator

dburles commented Mar 16, 2016

Sounds like you're building something a bit different overall to what the core of this package provides, I think it makes sense for you to build it yourself. I don't see any need to mimic the same name and API as here though if it's providing a different feature set.

@ivansglazunov
Copy link
Author

The task is one and the same. Namespace for collections.

@dburles
Copy link
Collaborator

dburles commented Mar 16, 2016

Remember the typical use case that I've seen with this package is usually for some kind of admin interface or low-level tool where they require the ability to show a list of collections. What's the use-case(s) for the extra features you're asking for? You say most people will have a use for them, but why? That's what I'm interested in.

@ivansglazunov
Copy link
Author

So there are two options:

  • Cut support null-collections completely.
  • Provide this support.

@ivansglazunov
Copy link
Author

It is not a question of my projects or applications. It is only a matter of null-collections.
Without ref possibility of their support is virtually nonexistent.
I can make the package mongo-collection-null-instances extendsMongo.Colleciton.get and Mongo.Collection.getAll and adding a separate arraynullInstances. Then this package can be completely cut out these possibilities.

@ivansglazunov
Copy link
Author

And a separate package mongo-collection-instances-ref add field _ref.

@dburles
Copy link
Collaborator

dburles commented Mar 16, 2016

Okay, so null collections are not currently supported, that we both agree on, but as I said earlier I have never seen an issue pop up relating to this being a problem and therefore solving a problem that doesn't exist is never a great idea.

You seem to have a requirement to have the ability to reference null collections, but you still haven't explained the practical reason why that would be useful. The reason why I ask is that I've never had the need to grab the collection based on a document fetched from it (shuttler:ref), and I've worked on a lot of Meteor applications!

@ivansglazunov
Copy link
Author

As an example:Unload from the server 2-5 server-collections of 50-100 documents. Edit them, without synchronization. Save thereafter.
Keep the history of changes in real time, but not on the server.
When you create a game with only 25 collections storage for ai. Some of them are virtual and are calculated based on the client server.
Within these client collections so the same is necessary to refer to each other.
It is not difficult to think up examples for simple sites stores.


I do not mind divide that on several packages, in principle, already have an understanding of how to do it. This way you think is correct?

@ivansglazunov
Copy link
Author

In any case, thanks for improved performance. ;)

@dburles
Copy link
Collaborator

dburles commented Mar 16, 2016

Really this is such a simple package (especially now since it's using collection-extensions) that I feel like if you run into a requirement like yours, it's simple enough for anyone to hack together their own version of it specific to their application and then just use that instead.

Though if you were to publish your own version, since the features would be different it would make sense to publish under a different name as to not confuse people with regards to their function. I think I'd just place all of the functionality within the shuttler:ref package.

The performance improvement PR makes sense, I'll look into getting that merged soon, thanks!

@ivansglazunov
Copy link
Author

Yeah, I guess you're right. I do not need to create wrappers. Thank you!

@dburles dburles closed this Feb 14, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants