Use ValueTokenizerHelper in KeySplineConverter, make it allocation free

Author: h3xds1nz

src/Microsoft.DotNet.Wpf/src/PresentationCore/System/Windows/Media/Animation/KeySplineConverter.cs

@@ -2,26 +2,20 @@
 // The .NET Foundation licenses this file to you under the MIT license.
 // See the LICENSE file in the project root for more information.
 
-//
-
-// Allow suppression of certain presharp messages
-#pragma warning disable 1634, 1691
-
-using MS.Internal;
-using System;
-using System.ComponentModel;
 using System.ComponentModel.Design.Serialization;
+using System.Windows.Media.Animation;
+using System.ComponentModel;
 using System.Globalization;
 using System.Reflection;
-using System.Windows.Media.Animation;
-using System.Security;
+using MS.Internal;
 
-using SR=MS.Internal.PresentationCore.SR;
+using SR = MS.Internal.PresentationCore.SR;
+using System.Runtime.CompilerServices;
 
 namespace System.Windows
 {
     /// <summary>
-    /// PointConverter - Converter class for converting instances of other types to Point instances
+    /// Converter class for converting instances of <see cref="KeySpline"/> to <see cref="string"/> and vice versa.
     /// </summary>
     /// <ExternalAPI/> 
     public class KeySplineConverter : TypeConverter
@@ -32,14 +26,7 @@ public class KeySplineConverter : TypeConverter
         /// <ExternalAPI/>
         public override bool CanConvertFrom(ITypeDescriptorContext typeDescriptor, Type destinationType)
         {
-            if (destinationType == typeof(string))
-            {
-                return true;
-            }
-            else
-            {
-                return false;
-            }
+            return destinationType == typeof(string);
         }
 
         /// <summary>
@@ -51,39 +38,23 @@ public override bool CanConvertFrom(ITypeDescriptorContext typeDescriptor, Type
         /// <ExternalAPI/>
         public override bool CanConvertTo(ITypeDescriptorContext context, Type destinationType)
         {
-            if (   destinationType == typeof(InstanceDescriptor)
-                || destinationType == typeof(string))
-            {
-                return true;
-            }
-            else
-            {
-                return false;
-            }
+            return destinationType == typeof(InstanceDescriptor) || destinationType == typeof(string);
         }
 
         /// <summary>
         /// ConvertFrom
         /// </summary>
-        public override object ConvertFrom(
-            ITypeDescriptorContext context, 
-            CultureInfo cultureInfo, 
-            object value)
+        public override object ConvertFrom(ITypeDescriptorContext context, CultureInfo cultureInfo, object value)
         {
-            string stringValue = value as string;
-
-            if (value == null)
-            {
+            if (value is not string stringValue)
                 throw new NotSupportedException(SR.Converter_ConvertFromNotSupported);
-            }
 
-            TokenizerHelper th = new TokenizerHelper(stringValue, cultureInfo);
+            ValueTokenizerHelper tokenizer = new(stringValue, cultureInfo);
 
-            return new KeySpline(
-                Convert.ToDouble(th.NextTokenRequired(), cultureInfo),
-                Convert.ToDouble(th.NextTokenRequired(), cultureInfo),
-                Convert.ToDouble(th.NextTokenRequired(), cultureInfo),
-                Convert.ToDouble(th.NextTokenRequired(), cultureInfo));
+            return new KeySpline(double.Parse(tokenizer.NextTokenRequired(), cultureInfo),
+                                 double.Parse(tokenizer.NextTokenRequired(), cultureInfo),
+                                 double.Parse(tokenizer.NextTokenRequired(), cultureInfo),
+                                 double.Parse(tokenizer.NextTokenRequired(), cultureInfo));
         }
 
         /// <summary>
@@ -95,47 +66,51 @@ public override object ConvertFrom(
         /// <param name="destinationType">Type to convert to</param>
         /// <returns>converted value</returns>
         /// <ExternalAPI/>
-        public override object ConvertTo(
-            ITypeDescriptorContext context, 
-            CultureInfo cultureInfo,
-            object value, 
-            Type destinationType)
+        public override object ConvertTo(ITypeDescriptorContext context, CultureInfo cultureInfo, object value, Type destinationType)
         {
-            KeySpline keySpline = value as KeySpline;
-
-            if (keySpline != null && destinationType != null)
+            if (value is KeySpline keySpline && destinationType is not null)
             {
+                if (destinationType == typeof(string))
+                    return ToString(keySpline, cultureInfo);
+
                 if (destinationType == typeof(InstanceDescriptor))
                 {
-                    ConstructorInfo ci = typeof(KeySpline).GetConstructor(new Type[] 
-                        {
-                            typeof(double), typeof(double),
-                            typeof(double), typeof(double) 
-                        });
-
-                    return new InstanceDescriptor(ci, new object[]
-                        {
-                            keySpline.ControlPoint1.X, keySpline.ControlPoint1.Y,
-                            keySpline.ControlPoint2.X, keySpline.ControlPoint2.Y
-                        });
-                }
-                else if (destinationType == typeof(string))
-                {
-#pragma warning disable 56506 // Suppress presharp warning: Parameter 'cultureInfo.TextInfo' to this public method must be validated:  A null-dereference can occur here.
-                    return String.Format(
-                        cultureInfo,
-                        "{0}{4}{1}{4}{2}{4}{3}",
-                        keySpline.ControlPoint1.X,
-                        keySpline.ControlPoint1.Y,
-                        keySpline.ControlPoint2.X,
-                        keySpline.ControlPoint2.Y,
-                        cultureInfo != null ? cultureInfo.TextInfo.ListSeparator : CultureInfo.InvariantCulture.TextInfo.ListSeparator);
-#pragma warning restore 56506
+                    ConstructorInfo ci = typeof(KeySpline).GetConstructor(new Type[] { typeof(double), typeof(double), typeof(double), typeof(double) });
+                    return new InstanceDescriptor(ci, new object[] { keySpline.ControlPoint1.X, keySpline.ControlPoint1.Y, keySpline.ControlPoint2.X, keySpline.ControlPoint2.Y });
                 }
             }
 
-            // Pass unhandled cases to base class (which will throw exceptions for null value or destinationType.)
+            // Pass unhandled cases to base class (which will throw exceptions for null value or destinationType)
             return base.ConvertTo(context, cultureInfo, value, destinationType);
         }
+
+        /// <summary>
+        /// Converts <paramref name="keySpline"/> to its string representation using the specified <paramref name="cultureInfo"/>.
+        /// </summary>
+        /// <param name="keySpline">The <see cref="KeySpline"/> to convert to string.</param>
+        /// <param name="cultureInfo">Culture to use when formatting doubles and choosing separator.</param>
+        /// <returns>The formatted <paramref name="keySpline"/> as string using the specified <paramref name="cultureInfo"/>.</returns>
+        private static string ToString(KeySpline keySpline, CultureInfo cultureInfo)
+        {
+            string listSeparator = cultureInfo != null ? cultureInfo.TextInfo.ListSeparator : CultureInfo.InvariantCulture.TextInfo.ListSeparator;
+
+            // Initial capacity [64] is an estimate based on a sum of:
+            // 48 = 4x double (fourteen digits is generous for the range of values likely)
+            //  3 = 3x separator characters
+            //  1 = 1x scratch space for alignment
+            DefaultInterpolatedStringHandler handler = new(3, 4, cultureInfo, stackalloc char[64]);
+            handler.AppendFormatted(keySpline.ControlPoint1.X);
+            handler.AppendLiteral(listSeparator);
+
+            handler.AppendFormatted(keySpline.ControlPoint1.Y);
+            handler.AppendLiteral(listSeparator);
+
+            handler.AppendFormatted(keySpline.ControlPoint2.X);
+            handler.AppendLiteral(listSeparator);
+
+            handler.AppendFormatted(keySpline.ControlPoint2.Y);
+
+            return handler.ToStringAndClear();
+        }
     }
 }