-
Notifications
You must be signed in to change notification settings - Fork 4.7k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[API Proposal]: Add methods which works with Guid/TimeSpan in System.Xml.XmlWriter/XmlReader #75515
Comments
Tagging subscribers to this area: @dotnet/area-system-xml Issue DetailsBackground and motivationWriting Guids and TimeSpans in xml is quire normal practice these days. We should allow users to use it instead of casting them to sring and then using WriteValue(string) method or using WriteValue(object). API Proposalnamespace System.Xml;
public class XmlWriter
{
public virtual void WriteValue(Guid value);
public virtual void WriteValue(TimeSpan value);
} API UsageXmlWriter writer;
writer.WriteValue(Guid.NewGuid());
writer.WriteValue(TimeSpan.FromHours(1)); Alternative DesignsWe can make these methods internal and use them only in System.Private.Xml project. Risksnone
|
Perhaps we could simply add a |
Not exactly as we want to format things in a specific way . For example Timespan is serialized like this right now : public static string ToString(TimeSpan value)
{
return new XsdDuration(value).ToString();
} |
Both |
@eiriktsarpalis I agree, just the clients should have some internal knowledge.
|
Personally, I see good use for these proposed overloads (and I'd even add overloads for DateOnly and TimeOnly). |
I have no strong opinion on adding @krwq do you think they're worth adding? |
I would just add that the format of |
Main benefit will be in writing Guids in XmlWriter as we will save Guid.ToString() allocation. |
@TrayanZapryanov would it be possible to update the API proposal in the OP to include |
@eiriktsarpalis Done. |
Thanks. I'm deferring to @krwq on whether to mark this as |
Small risk that I've found right now: // Writes out the specified value.
public virtual void WriteValue(object value)
{
ArgumentNullException.ThrowIfNull(value);
WriteString(XmlUntypedConverter.Untyped.ToString(value, null));
} and as XmlUntypedConverter.Untyped.ToString(value, null) is not supporting Guid - it will throw exception. // Writes out the specified value.
public virtual void WriteValue(Guid value)
{
WriteString(XmlConvert.ToString(value));
} |
One more note: |
I'm curious - do we actually care about XmlReader APIs or we just want XmlSerializer work with those? Are you actually using reader/writer directly? |
@krwq Nope, I am not using them. Proposal is just to use them when serializing primitive types in XmlSerializer. |
I think we should start with trying to handle that in XmlSerializer directly and only add APIs to XmlReader/XmlWriter if there is clear benefit |
@krwq You are right. I've added new performance issue. |
I'm not the owner of serialization part of the XML but I'd suggest running profiler with some real-life like payload and seeing if making a fix makes any difference. If there is clear difference then you already got prototype 😄 if change is relatively simple might be still worth to send a patch - I'd personally only do it if my particular scenario I'm trying to improve requires it (i.e. I have app which I'm trying to improve and profiler tells me this has huge impact on it or I'm particularly focusing on allocations/perf in XML) |
Background and motivation
Reading and writing Guids and TimeSpans in xml is quire normal practice these days.
We should allow users to read or write these primitive types instead of using existing ones based on string or object.
API Proposal
API Usage
Alternative Designs
We can make these methods internal and use them only in System.Private.Xml project.
Risks
none
The text was updated successfully, but these errors were encountered: