[CSHARP-509] Remove superfluous Create methods and change how remaining Create methods handle C# null Created: 22/Jun/12 Updated: 22/May/15 Resolved: 21/Apr/14 |
|
| Status: | Closed |
| Project: | C# Driver |
| Component/s: | None |
| Affects Version/s: | 1.8 |
| Fix Version/s: | 2.0 |
| Type: | Improvement | Priority: | Major - P3 |
| Reporter: | Brian | Assignee: | Robert Stam |
| Resolution: | Done | Votes: | 0 |
| Labels: | None | ||
| Remaining Estimate: | Not Specified | ||
| Time Spent: | Not Specified | ||
| Original Estimate: | Not Specified | ||
| Issue Links: |
|
||||||||||||||||
| Description |
|
[Original description] var value = BsonBoolean.Create("false"); value.RawValue == true ?!!! [Edited description] The BsonDocument object model has many Create methods that are not strictly required. We are going to do some cleaning up and remove them. There are explicit conversions that do exactly the same thing. We are leaving the Create methods that take an object parameter, because in that case the user can't call a constructor, because the required BsonValue subclass isn't known at compile time. We are also changing how the remaining Create methods handle C# null. They will now throw an ArgumentNullException if the value is C# null, with the exception of BsonValue.Create which will map C# null to BsonNull.Value. We are also changing the Add methods to throw ArgumentNullException if callled with a C# null argument (they used to treat C# null as a no-op). |
| Comments |
| Comment by Githook User [ 21/Apr/14 ] | |
|
Author: {u'name': u'rstam', u'email': u'robert@10gen.com'}Message: | |
| Comment by Githook User [ 21/Apr/14 ] | |
|
Author: {u'name': u'rstam', u'email': u'robert@10gen.com'}Message: | |
| Comment by Githook User [ 21/Apr/14 ] | |
|
Author: {u'name': u'rstam', u'email': u'robert@10gen.com'}Message: | |
| Comment by Githook User [ 21/Apr/14 ] | |
|
Author: {u'name': u'rstam', u'email': u'robert@10gen.com'}Message: | |
| Comment by Githook User [ 21/Apr/14 ] | |
|
Author: {u'name': u'rstam', u'email': u'robert@10gen.com'}Message: | |
| Comment by Robert Stam [ 24/Feb/13 ] | |
|
This work was prototyped on the experimental x2.0 branch. This ticket is being reopened for eventual porting to the official 2.0 branch. | |
| Comment by auto [ 10/Sep/12 ] | |
|
Author: {u'date': u'2012-09-10T14:54:59-07:00', u'email': u'craiggwilson@gmail.com', u'name': u'Craig Wilson'}Message: | |
| Comment by Robert Stam [ 10/Sep/12 ] | |
|
Fixed as part of some work on the linked JIRA issues. | |
| Comment by Robert Stam [ 10/Sep/12 ] | |
|
Now that we have removed most Create methods in favor of using the constructors directly and removed the special handling of C# null in functional construction (where it used to mean skip this value), we have also now changed the remaining Create methods to map C# null to BsonNull.Value, since we no longer depend on the Create methods letting C# null flow through them. So now BsonBoolean.Create(null) will throw an exception because C# null can't be mapped to a return type of BsonBoolean, but BsonValue.Create(null) will return BsonNull.Value, which does in fact return false when ToBoolean() is called. | |
| Comment by Robert Stam [ 10/Jul/12 ] | |
|
Changed the summary description to reflect that while the issue was initially filed for BsonBoolean.Create("false") it evolved into the more general question of what BsonBoolean.Create(null) should return. | |
| Comment by Robert Stam [ 10/Jul/12 ] | |
|
You make a good point, but currently all the BsonXyz.Create methods return C# null when passed a C# null, and we would have to change all of them to be consistent. This would also require changing the return type of BsonBoolean.Create from BsonBoolean to BsonValue because otherwise BsonNull.Value would be an invalid return value (and similarly for all the other Create methods). I'm leaving this open but rescheduling it to 2.0 so that more time can be given to considering what (if anything) we should change. Also, if this ends up requiring breaking changes then 2.0 is a good time for it since 2.0 will have lots of other breaking changes. | |
| Comment by Brian [ 22/Jun/12 ] | |
|
I got it. But then BsonBoolean.Create(null) must be false | |
| Comment by Robert Stam [ 22/Jun/12 ] | |
|
I can see why this is confusing, but here's the reasoning behind it:
If you actually want to parse the string instead of convert it using JavaScript's definition of truthiness, then you can write:
| |
| Comment by Brian [ 22/Jun/12 ] | |
|
case Conversion.StringToBsonBoolean: Very good indeed assumption |