Skip to content

Conversation

@alexjamesbrown
Copy link
Contributor

If no collection name is specified, the collection name will be inferred from the type name.

Also added passing unit tests

…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
@craiggwilson
Copy link
Contributor

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...

@alexjamesbrown
Copy link
Contributor Author

No disrespect, but why over-complicate things?
If a user wants a "pretty" collection name (lowercased, pluralized etc...) they should specify it.

My patch removes the concreted requirement to HAVE to enter a collection name, that's all...

@alexjamesbrown
Copy link
Contributor Author

... 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

@craiggwilson
Copy link
Contributor

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)
{
return db.GetCollection(typeof(T).Name);
}

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.

@alexjamesbrown
Copy link
Contributor Author

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.

@craiggwilson
Copy link
Contributor

https://github.com/mongodb/mongo-csharp-driver/blob/master/Bson/Serialization/Conventions/ConventionProfile.cs

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).

@rstam
Copy link
Contributor

rstam commented Aug 24, 2012

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:

  1. There are lots of conventions for naming collections (upper/lower case, pluralization, ...). Which to use?
  2. A collection may hold many classes (typically, but not necessarily, in a polymorphic inheritance hierarchy)
  3. It requires doubling the number of overloads of GetCollection (there are already 8)
  4. We can't use optional parameters yet (we're sticking with .NET Framework 3.5 for now)
  5. Conventions can vary between programming languages and many shops are multi language

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.

@alexjamesbrown
Copy link
Contributor Author

Agree with all the above points, which is why my patch, although simple, gave the user the simplest choice.
It's kind of annoying to have to keep specifying the collection name every time.

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!

@craiggwilson
Copy link
Contributor

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.
(4) - a reason for (3)
(5) - not sure why this matters. Use or create a convention that matches your desired outcome.

(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.

@alexjamesbrown
Copy link
Contributor Author

Yeah, #2 is the tricky part..
However, going back to my initial implementation, that puts the onus on the developer to know they're getting it right I guess.
I'm kinda split over that...

A "safeguard" could be to only allow that to happen if the type in GetCollection wasn't polymorphic (would need reflection to ensure that)

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.

3 participants