[CSHARP-467] Default values should only be applied if specified. Created: 08/May/12 Updated: 02/Apr/15 Resolved: 09/May/12 |
|
| Status: | Closed |
| Project: | C# Driver |
| Component/s: | None |
| Affects Version/s: | 1.4.2 |
| Fix Version/s: | 1.5 |
| Type: | Bug | Priority: | Major - P3 |
| Reporter: | Craig Wilson | Assignee: | Craig Wilson |
| Resolution: | Done | Votes: | 0 |
| Labels: | None | ||
| Remaining Estimate: | Not Specified | ||
| Time Spent: | Not Specified | ||
| Original Estimate: | Not Specified | ||
| Issue Links: |
|
||||||||
| Backwards Compatibility: | Minor Change | ||||||||
| Description |
|
A change was introduced somewhere along the way that applies default values to members even if they weren't explicitly set. We should leave members alone during deserialization if they haven't had a default value explicity set. |
| Comments |
| Comment by Craig Wilson [ 09/May/12 ] | |||||||||||
|
Sounds good Bas. That was my thinking as well. This has been merged to master. | |||||||||||
| Comment by Bas Geertsema [ 09/May/12 ] | |||||||||||
|
Here is the tracked down reporter I assume that there are two use cases for using bson default values. 1) Upon deserialization to provide values for fields that do not exist in the database document and 2) Upon serialization to ignore default values and ignore data usage. In the second case there is no ambiguousness and itis merely a convenience to increase performance. In the first case there is in the deserialization of an object of class C some ambiguousness because the specified default value is different in the constructor and the attribute. If both default values are specified in the class definition then this is just bad design imho. But a more reasonable design would be if an application developer would use a third party object and then specify default values for this object using class maps. In the latter case the default values set by the application developer are more specific to the application at hand than the constructor and therefore imho should have precedence over the default value set in the constructor. | |||||||||||
| Comment by Craig Wilson [ 09/May/12 ] | |||||||||||
|
Great question!!! And... I don't know. Currently, it will set X to 2. Perhaps, before applying a default value, we could check to see if the member has been altered from it's default .NET value and, if so, either not set it or throw an exception? I'd like to get our reporters view on this. I'll go track him down. | |||||||||||
| Comment by Robert Stam [ 09/May/12 ] | |||||||||||
|
OK, I'm convinced... Can you think about this edge case and decide what the right action is:
Thanks. | |||||||||||
| Comment by Craig Wilson [ 09/May/12 ] | |||||||||||
|
We will set members that are missing elements to their default value if one is specified. Otherwise, we don't set anything at all. In the case where we don't set anything, .NET will initialize it to the default value anyways, so that isn't an issue. However, what we will not do is overwrite a value set in the constructor with an unspecified default value. So, yes, we might end up with a document in memory that doesn't match the database because someone chose to initialize a member in the constructor. However, that makes way more sense than what is being done currently, which is to overwrite everything that happens in the constructor. Philisophy Warning | |||||||||||
| Comment by Robert Stam [ 09/May/12 ] | |||||||||||
|
This is debatable. By not setting field or properties that are missing elements to their default value you can end up with a document in memory that is not the same as the document in the database. If you then save the document back out you end up with a different document. | |||||||||||
| Comment by Craig Wilson [ 08/May/12 ] | |||||||||||
|
I've sent in pull request 107 at github for this issue: https://github.com/mongodb/mongo-csharp-driver/pull/107. Should be included in 1.5. |