Handle null values in XML as well #49
Conversation
This is the same as what we do for JSON, we make it possible to leave out values that are null, which is important for certain special values like UAString, where an empty value is different from a null value. I considered making this a separate trait, but it would be annoying to have to derive a trait that does nothing most of the time, and without specialization there's no better way. We also can't put it on BinaryEncodable since that trait isn't implemented for `Option`, and can't really be at the moment. It's now effectively duplicated between JsonEncodable and XmlEncodable, which is fine.
einarmo
force-pushed
the
xml-handle-null
branch
from
c3e2494 to
3771cf4
Compare
| /// This method should return `true` if the value is default | ||
| /// and should not be serialized. | ||
| fn is_null_xml(&self) -> bool { | ||
| false |
There was a problem hiding this comment.
Guess this method will never return true. Is that expected?
There was a problem hiding this comment.
This is the trait default, it's overridden in certain types.
There was a problem hiding this comment.
As it's defined in a trait, wouldn't it be better to not implement it as a default false but require it to be implemented by anyone that implements this trait for a type?
There was a problem hiding this comment.
We could do that, I guess. It would be a lot of noise though, and most implementations would simply be false.
If we do this, I'd rather do it separately, since we would want to do the same for JSON. Maybe combine it with moving this to a separate trait?
There was a problem hiding this comment.
Just wondered if this could cause unexpected behavior were a type forgets to implement/overwrite the default. But as the default is false I guess that would be unlikely.
| /// This method should return `true` if the value is default | ||
| /// and should not be serialized. | ||
| fn is_null_xml(&self) -> bool { | ||
| false |
There was a problem hiding this comment.
Just wondered if this could cause unexpected behavior were a type forgets to implement/overwrite the default. But as the default is false I guess that would be unlikely.
|
It's not an entirely unreasonable thought, and it's a danger of default implementations. In this case, leaving it The only real exception is <String></String>and <String />mean different things. One is an empty string, the other is a null string. Since the type only governs the content of the tag in our XML serializer, it means we can deserialize the latter, and serialize that back as the first without this feature. For every other type, as well as for all types in JSON, the only advantage is strict standards compliance (the standard wants you to leave out "null" values in some cases), and data size reduction. |
This is the same as what we do for JSON, we make it possible to leave out values that are null, which is important for certain special values like UAString, where an empty value is different from a null value.
I considered making this a separate trait, but it would be annoying to have to derive a trait that does nothing most of the time, and without specialization there's no better way. We also can't put it on BinaryEncodable since that trait isn't implemented for
Option, and can't really be at the moment. It's now effectively duplicated between JsonEncodable and XmlEncodable, which is fine.