From b6695a8cc8aea9d4a696c762d8cd1fde493b6e12 Mon Sep 17 00:00:00 2001 From: kronic Date: Wed, 18 Aug 2021 18:02:45 +0300 Subject: [PATCH 1/4] Refactor --- .../src/System/Xml/XmlQualifiedName.cs | 79 +++++++------------ 1 file changed, 30 insertions(+), 49 deletions(-) diff --git a/src/libraries/System.Private.Xml/src/System/Xml/XmlQualifiedName.cs b/src/libraries/System.Private.Xml/src/System/Xml/XmlQualifiedName.cs index 63ca7e274c53f..08ff51ea5b26e 100644 --- a/src/libraries/System.Private.Xml/src/System/Xml/XmlQualifiedName.cs +++ b/src/libraries/System.Private.Xml/src/System/Xml/XmlQualifiedName.cs @@ -12,14 +12,12 @@ namespace System.Xml /// public class XmlQualifiedName { - private string _name; - private string _ns; private int _hash; /// /// [To be supplied.] /// - public static readonly XmlQualifiedName Empty = new XmlQualifiedName(string.Empty); + public static readonly XmlQualifiedName Empty = new(string.Empty); /// /// [To be supplied.] @@ -36,26 +34,18 @@ public XmlQualifiedName(string? name) : this(name, string.Empty) { } /// public XmlQualifiedName(string? name, string? ns) { - _ns = ns ?? string.Empty; - _name = name ?? string.Empty; + Namespace = ns ?? string.Empty; + Name = name ?? string.Empty; } /// /// [To be supplied.] /// - public string Namespace - { - get { return _ns; } - } - + public string Namespace { get; private set; } /// /// [To be supplied.] /// - public string Name - { - get { return _name; } - } - + public string Name { get; private set; } /// /// [To be supplied.] /// @@ -63,7 +53,7 @@ public override int GetHashCode() { if (_hash == 0) { - _hash = Name.GetHashCode() /*+ Namespace.GetHashCode()*/; // for perf reasons we are not taking ns's hashcode. + _hash = Name.GetHashCode(); /*+ Namespace.GetHashCode()*/ // for perf reasons we are not taking ns's hashcode. } return _hash; } @@ -71,17 +61,13 @@ public override int GetHashCode() /// /// [To be supplied.] /// - public bool IsEmpty - { - get { return Name.Length == 0 && Namespace.Length == 0; } - } - + public bool IsEmpty => Name.Length == 0 && Namespace.Length == 0; /// /// [To be supplied.] /// public override string ToString() { - return Namespace.Length == 0 ? Name : string.Concat(Namespace, ":", Name); + return Namespace.Length == 0 ? Name : $"{Namespace}:{Name}"; } /// @@ -89,18 +75,12 @@ public override string ToString() /// public override bool Equals([NotNullWhen(true)] object? other) { - if ((object)this == other) + if (ReferenceEquals(this, other)) { return true; } - XmlQualifiedName? qname = other as XmlQualifiedName; - if (qname != null) - { - return (Name == qname.Name && Namespace == qname.Namespace); - } - - return false; + return other is XmlQualifiedName qName && Name == qName.Name && Namespace == qName.Namespace; } /// @@ -108,11 +88,15 @@ public override bool Equals([NotNullWhen(true)] object? other) /// public static bool operator ==(XmlQualifiedName? a, XmlQualifiedName? b) { - if ((object?)a == (object?)b) + if (ReferenceEquals(a, b)) + { return true; + } - if (a is null || b is null) + if (ReferenceEquals(a, null) || ReferenceEquals(b, null)) + { return false; + } return a.Name == b.Name && a.Namespace == b.Namespace; } @@ -130,42 +114,41 @@ public override bool Equals([NotNullWhen(true)] object? other) /// public static string ToString(string name, string ns) { - return ns == null || ns.Length == 0 ? name : ns + ":" + name; + return ns == null || ns.Length == 0 ? name : $"{ns}:{name}"; } // --------- Some useful internal stuff ----------------- internal void Init(string? name, string? ns) { - _name = name ?? string.Empty; - _ns = ns ?? string.Empty; + Name = name ?? string.Empty; + Namespace = ns ?? string.Empty; _hash = 0; } internal void SetNamespace(string? ns) { - _ns = ns ?? string.Empty; // Not changing hash since ns is not used to compute hashcode + Namespace = ns ?? string.Empty; // Not changing hash since ns is not used to compute hashcode } internal void Verify() { - XmlConvert.VerifyNCName(_name); - if (_ns.Length != 0) + XmlConvert.VerifyNCName(Name); + if (Namespace.Length != 0) { - XmlConvert.ToUri(_ns); + XmlConvert.ToUri(Namespace); } } internal void Atomize(XmlNameTable nameTable) { - Debug.Assert(_name != null); - _name = nameTable.Add(_name); - _ns = nameTable.Add(_ns); + Debug.Assert(Name != null); + Name = nameTable.Add(Name); + Namespace = nameTable.Add(Namespace); } internal static XmlQualifiedName Parse(string s, IXmlNamespaceResolver nsmgr, out string prefix) { - string localName; - ValidateNames.ParseQNameThrow(s, out prefix, out localName); + ValidateNames.ParseQNameThrow(s, out prefix, out string localName); string? uri = nsmgr.LookupNamespace(prefix); if (uri == null) @@ -174,11 +157,9 @@ internal static XmlQualifiedName Parse(string s, IXmlNamespaceResolver nsmgr, ou { throw new XmlException(SR.Xml_UnknownNs, prefix); } - else - { - // Re-map namespace of empty prefix to string.Empty when there is no default namespace declared - uri = string.Empty; - } + + // Re-map namespace of empty prefix to string.Empty when there is no default namespace declared + uri = string.Empty; } return new XmlQualifiedName(localName, uri); From a43bacc86af3e63cd3e1f6aac38825828bde1436 Mon Sep 17 00:00:00 2001 From: kronic Date: Thu, 19 Aug 2021 09:32:11 +0300 Subject: [PATCH 2/4] feedback --- .../System.Private.Xml/src/System/Xml/XmlQualifiedName.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libraries/System.Private.Xml/src/System/Xml/XmlQualifiedName.cs b/src/libraries/System.Private.Xml/src/System/Xml/XmlQualifiedName.cs index 08ff51ea5b26e..fcd77f4a14b9d 100644 --- a/src/libraries/System.Private.Xml/src/System/Xml/XmlQualifiedName.cs +++ b/src/libraries/System.Private.Xml/src/System/Xml/XmlQualifiedName.cs @@ -93,7 +93,7 @@ public override bool Equals([NotNullWhen(true)] object? other) return true; } - if (ReferenceEquals(a, null) || ReferenceEquals(b, null)) + if (a is null || b is null) { return false; } From 0b0502d9d1ea9a0d6ebd392c8c33f378d6f72415 Mon Sep 17 00:00:00 2001 From: kronic Date: Thu, 19 Aug 2021 18:22:03 +0300 Subject: [PATCH 3/4] feedback --- .../System.Private.Xml/src/System/Xml/XmlQualifiedName.cs | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/src/libraries/System.Private.Xml/src/System/Xml/XmlQualifiedName.cs b/src/libraries/System.Private.Xml/src/System/Xml/XmlQualifiedName.cs index fcd77f4a14b9d..e9041a344ab6a 100644 --- a/src/libraries/System.Private.Xml/src/System/Xml/XmlQualifiedName.cs +++ b/src/libraries/System.Private.Xml/src/System/Xml/XmlQualifiedName.cs @@ -1,8 +1,6 @@ // Licensed to the .NET Foundation under one or more agreements. // The .NET Foundation licenses this file to you under the MIT license. -using System.Collections; -using System.Diagnostics; using System.Diagnostics.CodeAnalysis; namespace System.Xml @@ -112,7 +110,7 @@ public override bool Equals([NotNullWhen(true)] object? other) /// /// [To be supplied.] /// - public static string ToString(string name, string ns) + public static string ToString(string name, string? ns) { return ns == null || ns.Length == 0 ? name : $"{ns}:{name}"; } @@ -141,7 +139,6 @@ internal void Verify() internal void Atomize(XmlNameTable nameTable) { - Debug.Assert(Name != null); Name = nameTable.Add(Name); Namespace = nameTable.Add(Namespace); } From 0240ef8085889664d7e1610f71e70781d2d05292 Mon Sep 17 00:00:00 2001 From: kronic Date: Thu, 19 Aug 2021 23:19:06 +0300 Subject: [PATCH 4/4] done --- .../System.Xml.ReaderWriter/ref/System.Xml.ReaderWriter.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libraries/System.Xml.ReaderWriter/ref/System.Xml.ReaderWriter.cs b/src/libraries/System.Xml.ReaderWriter/ref/System.Xml.ReaderWriter.cs index 91f9f48862f1e..a576ee8439dfe 100644 --- a/src/libraries/System.Xml.ReaderWriter/ref/System.Xml.ReaderWriter.cs +++ b/src/libraries/System.Xml.ReaderWriter/ref/System.Xml.ReaderWriter.cs @@ -765,7 +765,7 @@ public XmlQualifiedName(string? name, string? ns) { } public static bool operator ==(System.Xml.XmlQualifiedName? a, System.Xml.XmlQualifiedName? b) { throw null; } public static bool operator !=(System.Xml.XmlQualifiedName? a, System.Xml.XmlQualifiedName? b) { throw null; } public override string ToString() { throw null; } - public static string ToString(string name, string ns) { throw null; } + public static string ToString(string name, string? ns) { throw null; } } [System.Diagnostics.DebuggerDisplayAttribute("{debuggerDisplayProxy}")] [System.Diagnostics.DebuggerDisplayAttribute("{debuggerDisplayProxy}")]