-
Notifications
You must be signed in to change notification settings - Fork 1.3k
Changed GetCollection methods so we no longer have to supply a collection name #126
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
Changed GetCollection methods so we no longer have to supply a collection name #126
Conversation
…upply a collection name. If no collection name is specified, the collection name will be inferred from the type name. Also added passing unit tests
|
Thanks so much for the pull request. We really want to do this, but would like to do it in a much more comprehensive manner. This would probably include a collection name convention such that the default collection name could be lowercased, pluralized, etc... |
|
No disrespect, but why over-complicate things? My patch removes the concreted requirement to HAVE to enter a collection name, that's all... |
|
... but i certainly agree, the ability to do your suggestion would be great as well... but will obviously take longer, and would / could be an extension of the pull request I submitted |
|
No disrespect taken :) Agreed that it is more-complicated (although not by much... the pattern is already there and is relatively easy to follow). The reason we would like it to be more robust is because when we implement a feature, we'd like to not need to come back and re-implement it later because we only got 50% of people the first time. Also, your fix can be achieved by creating an extension method on MongoDatabase, in which case you can use this implementation without needing to pull it in. public static MongoCollection GetCollection(this MongoDatabase db) Like I said, we are very appreciative of the request and I hope this doesn't hinder you from making others when you feel a need. It might be that we pull this in as a starting point when the "full-feature" is implemented. |
|
That's essentially what i do in my app now, just wanted to make it part of the framework. if you point me in the right direction of where the pattern is already used i'll extend this and try and plug it in as you'd like. |
|
Basically, you'd create an 1) ICollectionNameConvention, 2) give it a default implementation, 3) include it in the convention profile, 4) add a property into BsonClassMap to read the collection name from the convention. At this point, the pattern diverges because now you have to mess with MongoDatabase. 5) Add overloads in MongoDatabase to get the BsonClassMap for the document in both CreateCollectionSettings and GetCollection. We have a long-outstanding JIRA issue for this (https://jira.mongodb.org/browse/CSHARP-126). Before you go do this, I'd like Robert (the project lead) to weigh in on whether he thinks this is the right approach. As I said, this isn't all that difficult to do, so I'm wondering why we haven't done it yet (what I'm missing). |
|
I haven't considered this to be much of a priority because it is not a clearly defined problem (and is it even a problem?). For example:
In my mind it is so easy to provide the collection name yourself that there is little to be gained by automating it, and automating it in a way that isn't generally correct is even worse. |
|
Agree with all the above points, which is why my patch, although simple, gave the user the simplest choice. GetCollection().AsQueryable is a lot nicer to write than GetCollection("User").AsQueryable Was about giving the developer the choice... not forcing them to have to specify a collection name every time, when sometimes, you just don't need to! |
|
Well the impetus is the same as with all "automation" solutions. Centralize the configuration, get rid of magic strings, etc... Regardless, I don't feel that it is a priority as there is a simple way of providing this (custom extension method). As I mention below, (2) is the real issue. (1) - isn't our problem. We'd provide a couple common ones, pick the default and go with it. (3) - simply a maintenance issue. But its also the overloads to CreateCollectionSettings methods, which adds an additional 3 overloads. (2) - is actually the problem as I see it. Getting it right could be pretty tricky. |
|
Yeah, #2 is the tricky part.. A "safeguard" could be to only allow that to happen if the type in GetCollection wasn't polymorphic (would need reflection to ensure that) |
If no collection name is specified, the collection name will be inferred from the type name.
Also added passing unit tests