[CSHARP-179] Add an Implicit cast from HashTable to BsonDocument to make it easier to use the driver in powershell Created: 04/Mar/11 Updated: 02/Apr/15 Resolved: 15/Mar/11 |
|
| Status: | Closed |
| Project: | C# Driver |
| Component/s: | None |
| Affects Version/s: | None |
| Fix Version/s: | 1.0 |
| Type: | Improvement | Priority: | Major - P3 |
| Reporter: | Justin Dearing | Assignee: | Robert Stam |
| Resolution: | Done | Votes: | 0 |
| Labels: | None | ||
| Remaining Estimate: | Not Specified | ||
| Time Spent: | Not Specified | ||
| Original Estimate: | Not Specified | ||
| Environment: |
Windows Powershell 2.0. Not tested in PS 1.0 |
||
| Description |
|
Powershell has great native support for HashTables via the @ { "key"="value; . . .}syntax.. This simple implicit cast allows you to take adtantage of this notation in Powershell to create MongoDocuments.. I also documented how to use this syntax in powershell. Commit: Screenshots of .chm file compiled with sandcastle: |
| Comments |
| Comment by Robert Stam [ 16/Mar/11 ] |
|
I was also wondering if some of the documentation comments could be put in a separate file, but have not looked into it. If you figure it out, let me know. Thanks! |
| Comment by Justin Dearing [ 16/Mar/11 ] |
|
Robert, My powershell script runs against the latest code, so yes the changes are ok. If there are any edge cases from changing ToString() to a cast, then the longer syntax will suffice. 99% of the powershell users will have the more convient syntax. With regards to the documentation, if its possible to store the examples section in an external XML file, would you be ok with that? Then we could have verbose documentation that doesn't clutter the .cs files. Justin |
| Comment by Robert Stam [ 15/Mar/11 ] |
|
I merged in your pull request and made some changes to follow the pattern set by the existing IDictionary overloads. 1. Two new constructor overloads (one allows you to specify the keys you want to load from the hash table) One change that MIGHT affect you is: it is now REQUIRED that the keys of the Hashtable already be strings. In other words, I am casting the keys to string instead of calling ToString. This is consistent with the IDictionary support, where the keys are also required to be strings. Let me know if you think this is a mistake (I think allowing arbitrary key types would be a mistake). Also, we haven't yet figured out the right balance for the amount of information to put in the doc comments. I'm leaning towards keeping the doc comments very lean. This is probably not the place to be putting extensive documentation, or it will make the source files really hard to read and to maintain. In any case, I believe the sample code in the doc comments was not updated to reflect the change from implicit conversion to explicit call of a constructor. I don't know enough about PowerShell syntax to fix that. |
| Comment by Justin Dearing [ 06/Mar/11 ] |
|
Are those two patches acceptable? While you are still stuck using reflection to use the CRUD moethods in MongoCollection, this makes assembling MongoDocuments much easier. |
| Comment by Justin Dearing [ 04/Mar/11 ] |
|
Updated the pull request. My new additional commit that makes the cast a constructor and improves documentation is here: |
| Comment by Justin Dearing [ 04/Mar/11 ] |
|
The constructor seems to work. I am futzing with sandcastle docs and I'll have new commit in a few. |
| Comment by Robert Stam [ 04/Mar/11 ] |
|
Try the constructor. It should be consistent with existing code (which is using constructors and not conversion operators). |
| Comment by Justin Dearing [ 04/Mar/11 ] |
|
Perhaps an alternative would be a duplicate key event that gets fired similar to event that gets fired when validating Xml to an XSD schema via an Xml reader (I'd have )to dig up code if. you don't know what I am talking about) Or, perhaps an option that in the event of a duplicate, make the property an array. Also, apparently Powershell will implicitly use explicit casts so changing the keyword from implicit to explicit will work. Is that good enough or should I try the constructor alternatives? |
| Comment by Robert Stam [ 04/Mar/11 ] |
|
Allowing duplicate key names usually causes nothing but trouble. The server doesn't officially support it (though it seems to work). It was primarily put in to allow easy mapping of XML to BSON, but not sure how useful that is if you then can't put the resulting document into the database anyway. |
| Comment by Justin Dearing [ 04/Mar/11 ] |
|
Robert, I'll see if I can find an alternative like a constructor that concise and pretty in PowerShell (which is my ONLY reason to care about Hashtable). Since I don't understand the point of a bson document with duplicate keys, do you want duplicate keys allowed? I actually prefer making people do that the hard way in powershell if there is a need for duplicate keys. Justin |
| Comment by Robert Stam [ 04/Mar/11 ] |
|
I would prefer a new constructor like the existing one for BsonDocument(IDictionary<string, object>). Say BsonDocument(IDictionary). Implicit conversions often get you in trouble because they can't be turned off and people often don't expect them to exist. Not necessarily saying we can't do it, but would prefer another alternative that was explicit. |
| Comment by Justin Dearing [ 04/Mar/11 ] |
|
Thanks to a git commit--amend to change rthe comment the correct patch is https://github.com/zippy1981/mongo-csharp-driver/commit/35d8f1f6b91510775b8d1d3e43d8d414966e7f68 |