Cleaner semantics for binders

- Binders avoid throwing exceptions where possible - Conversion exceptions are swallowed without calling setter - Failed conversions don't call the setter - Nullable allows 'empty' input, non-nullable does not This addresses problems that were found as a result of fixing error handling.
dotnet · Mar 19, 2019 · ca14bc7 · ca14bc7
1 parent 2ec2f4c
commit ca14bc7
Show file tree

Hide file tree

Showing 5 changed files with 456 additions and 54 deletions.
diff --git a/src/Components/Components/ref/Microsoft.AspNetCore.Components.netstandard2.0.cs b/src/Components/Components/ref/Microsoft.AspNetCore.Components.netstandard2.0.cs
@@ -155,7 +155,7 @@ public static partial class EventCallbackFactoryBinderExtensions
         public static Microsoft.AspNetCore.Components.EventCallback<Microsoft.AspNetCore.Components.UIChangeEventArgs> CreateBinder(this Microsoft.AspNetCore.Components.EventCallbackFactory factory, object receiver, System.Action<float?> setter, float? existingValue) { throw null; }
         public static Microsoft.AspNetCore.Components.EventCallback<Microsoft.AspNetCore.Components.UIChangeEventArgs> CreateBinder(this Microsoft.AspNetCore.Components.EventCallbackFactory factory, object receiver, System.Action<float> setter, float existingValue) { throw null; }
         public static Microsoft.AspNetCore.Components.EventCallback<Microsoft.AspNetCore.Components.UIChangeEventArgs> CreateBinder(this Microsoft.AspNetCore.Components.EventCallbackFactory factory, object receiver, System.Action<string> setter, string existingValue) { throw null; }
-        public static Microsoft.AspNetCore.Components.EventCallback<Microsoft.AspNetCore.Components.UIChangeEventArgs> CreateBinder<T>(this Microsoft.AspNetCore.Components.EventCallbackFactory factory, object receiver, System.Action<T> setter, T existingValue) where T : System.Enum { throw null; }
+        public static Microsoft.AspNetCore.Components.EventCallback<Microsoft.AspNetCore.Components.UIChangeEventArgs> CreateBinder<T>(this Microsoft.AspNetCore.Components.EventCallbackFactory factory, object receiver, System.Action<T> setter, T existingValue) where T : struct, System.Enum { throw null; }
     }
     public static partial class EventCallbackFactoryUIEventArgsExtensions
     {

diff --git a/src/Components/Components/src/EventCallbackFactoryBinderExtensions.cs b/src/Components/Components/src/EventCallbackFactoryBinderExtensions.cs
@@ -11,74 +11,263 @@ namespace Microsoft.AspNetCore.Components
     /// </summary>
     public static class EventCallbackFactoryBinderExtensions
     {
+        private delegate bool BindConverter<T>(object obj, out T value);
+
         // Perf: conversion delegates are written as static funcs so we can prevent
         // allocations for these simple cases.
-        private static Func<object, string> ConvertToString = (obj) => (string)obj;
+        private readonly static BindConverter<string> ConvertToString = ConvertToStringCore;
 
-        private static Func<object, bool> ConvertToBool = (obj) => (bool)obj;
-        private static Func<object, bool?> ConvertToNullableBool = (obj) => (bool?)obj;
+        private static bool ConvertToStringCore(object obj, out string value)
+        {
+            // We expect the input to already be a string.
+            value = (string)obj;
+            return true;
+        }
 
-        private static Func<object, int> ConvertToInt = (obj) => int.Parse((string)obj);
-        private static Func<object, int?> ConvertToNullableInt = (obj) =>
+        private static BindConverter<bool> ConvertToBool = ConvertToBoolCore;
+        private static BindConverter<bool?> ConvertToNullableBool = ConvertToNullableBoolCore;
+
+        private static bool ConvertToBoolCore(object obj, out bool value)
         {
-            if (int.TryParse((string)obj, out var value))
+            // We expect the input to already be a bool.
+            value = (bool)obj;
+            return true;
+        }
+
+        private static bool ConvertToNullableBoolCore(object obj, out bool? value)
+        {
+            // We expect the input to already be a bool.
+            value = (bool?)obj;
+            return true;
+        }
+
+        private static BindConverter<int> ConvertToInt = ConvertToIntCore;
+        private static BindConverter<int?> ConvertToNullableInt = ConvertToNullableIntCore;
+
+        private static bool ConvertToIntCore(object obj, out int value)
+        {
+            var text = (string)obj;
+            if (string.IsNullOrEmpty(text))
             {
-                return value;
+                value = default;
+                return false;
+            }
+
+            if (!int.TryParse(text, out var converted))
+            {
+                value = default;
+                return false;
             }
 
-            return null;
-        };
+            value = converted;
+            return true;
+        }
 
-        private static Func<object, long> ConvertToLong = (obj) => long.Parse((string)obj);
-        private static Func<object, long?> ConvertToNullableLong = (obj) =>
+        private static bool ConvertToNullableIntCore(object obj, out int? value)
         {
-            if (long.TryParse((string)obj, out var value))
+            var text = (string)obj;
+            if (string.IsNullOrEmpty(text))
             {
-                return value;
+                value = default;
+                return true;
             }
 
-            return null;
-        };
+            if (!int.TryParse(text, out var converted))
+            {
+                value = default;
+                return false;
+            }
+
+            value = converted;
+            return true;
+        }
+
+        private static BindConverter<long> ConvertToLong = ConvertToLongCore;
+        private static BindConverter<long?> ConvertToNullableLong = ConvertToNullableLongCore;
 
-        private static Func<object, float> ConvertToFloat = (obj) => float.Parse((string)obj);
-        private static Func<object, float?> ConvertToNullableFloat = (obj) =>
+        private static bool ConvertToLongCore(object obj, out long value)
         {
-            if (float.TryParse((string)obj, out var value))
+            var text = (string)obj;
+            if (string.IsNullOrEmpty(text))
             {
-                return value;
+                value = default;
+                return false;
+            }
+
+            if (!long.TryParse(text, out var converted))
+            {
+                value = default;
+                return false;
+            }
+
+            value = converted;
+            return true;
+        }
+
+        private static bool ConvertToNullableLongCore(object obj, out long? value)
+        {
+            var text = (string)obj;
+            if (string.IsNullOrEmpty(text))
+            {
+                value = default;
+                return true;
             }
 
-            return null;
-        };
+            if (!long.TryParse(text, out var converted))
+            {
+                value = default;
+                return false;
+            }
 
-        private static Func<object, double> ConvertToDouble = (obj) => double.Parse((string)obj);
-        private static Func<object, double?> ConvertToNullableDouble = (obj) =>
+            value = converted;
+            return true;
+        }
+
+        private static BindConverter<float> ConvertToFloat = ConvertToFloatCore;
+        private static BindConverter<float?> ConvertToNullableFloat = ConvertToNullableFloatCore;
+
+        private static bool ConvertToFloatCore(object obj, out float value)
         {
-            if (double.TryParse((string)obj, out var value))
+            var text = (string)obj;
+            if (string.IsNullOrEmpty(text))
             {
-                return value;
+                value = default;
+                return false;
+            }
+
+            if (!float.TryParse(text, out var converted))
+            {
+                value = default;
+                return false;
             }
 
-            return null;
-        };
+            value = converted;
+            return true;
+        }
 
-        private static Func<object, decimal> ConvertToDecimal = (obj) => decimal.Parse((string)obj);
-        private static Func<object, decimal?> ConvertToNullableDecimal = (obj) =>
+        private static bool ConvertToNullableFloatCore(object obj, out float? value)
         {
-            if (decimal.TryParse((string)obj, out var value))
+            var text = (string)obj;
+            if (string.IsNullOrEmpty(text))
             {
-                return value;
+                value = default;
+                return true;
             }
 
-            return null;
-        };
+            if (!float.TryParse(text, out var converted))
+            {
+                value = default;
+                return false;
+            }
+
+            value = converted;
+            return true;
+        }
+
+        private static BindConverter<double> ConvertToDouble = ConvertToDoubleCore;
+        private static BindConverter<double?> ConvertToNullableDouble = ConvertToNullableDoubleCore;
 
-        private static class EnumConverter<T> where T : Enum
+        private static bool ConvertToDoubleCore(object obj, out double value)
         {
-            public static Func<object, T> Convert = (obj) =>
+            var text = (string)obj;
+            if (string.IsNullOrEmpty(text))
             {
-                return (T)Enum.Parse(typeof(T), (string)obj);
-            };
+                value = default;
+                return false;
+            }
+
+            if (!double.TryParse(text, out var converted))
+            {
+                value = default;
+                return false;
+            }
+
+            value = converted;
+            return true;
+        }
+
+        private static bool ConvertToNullableDoubleCore(object obj, out double? value)
+        {
+            var text = (string)obj;
+            if (string.IsNullOrEmpty(text))
+            {
+                value = default;
+                return true;
+            }
+
+            if (!double.TryParse(text, out var converted))
+            {
+                value = default;
+                return false;
+            }
+
+            value = converted;
+            return true;
+        }
+
+        private static BindConverter<decimal> ConvertToDecimal = ConvertToDecimalCore;
+        private static BindConverter<decimal?> ConvertToNullableDecimal = ConvertToNullableDecimalCore;
+
+        private static bool ConvertToDecimalCore(object obj, out decimal value)
+        {
+            var text = (string)obj;
+            if (string.IsNullOrEmpty(text))
+            {
+                value = default;
+                return false;
+            }
+
+            if (!decimal.TryParse(text, out var converted))
+            {
+                value = default;
+                return false;
+            }
+
+            value = converted;
+            return true;
+        }
+
+        private static bool ConvertToNullableDecimalCore(object obj, out decimal? value)
+        {
+            var text = (string)obj;
+            if (string.IsNullOrEmpty(text))
+            {
+                value = default;
+                return true;
+            }
+
+            if (!decimal.TryParse(text, out var converted))
+            {
+                value = default;
+                return false;
+            }
+
+            value = converted;
+            return true;
+        }
+
+        private static class EnumConverter<T> where T : struct, Enum
+        {
+            public static readonly BindConverter<T> Convert = ConvertCore;
+
+            public static bool ConvertCore(object obj, out T value)
+            {
+                var text = (string)obj;
+                if (string.IsNullOrEmpty(text))
+                {
+                    value = default;
+                    return true;
+                }
+
+                if (!Enum.TryParse<T>(text, out var converted))
+                {
+                    value = default;
+                    return false;
+                }
+
+                value = converted;
+                return true;
+            }
         }
 
         /// <summary>
@@ -330,7 +519,22 @@ public static EventCallback<UIChangeEventArgs> CreateBinder(
             // when a format is used.
             Action<UIChangeEventArgs> callback = (e) =>
             {
-                setter(ConvertDateTime(e.Value, format: null));
+                DateTime value = default;
+                var converted = false;
+                try
+                {
+                    value = ConvertDateTime(e.Value, format: null);
+                    converted = true;
+                }
+                catch
+                {
+                }
+
+                // See comments in CreateBinderCore
+                if (converted)
+                {
+                    setter(value);
+                }
             };
             return factory.Create<UIChangeEventArgs>(receiver, callback);
         }
@@ -355,7 +559,22 @@ public static EventCallback<UIChangeEventArgs> CreateBinder(
             // when a format is used.
             Action<UIChangeEventArgs> callback = (e) =>
             {
-                setter(ConvertDateTime(e.Value, format));
+                DateTime value = default;
+                var converted = false;
+                try
+                {
+                    value = ConvertDateTime(e.Value, format);
+                    converted = true;
+                }
+                catch
+                {
+                }
+
+                // See comments in CreateBinderCore
+                if (converted)
+                {
+                    setter(value);
+                }
             };
             return factory.Create<UIChangeEventArgs>(receiver, callback);
         }
@@ -373,7 +592,7 @@ public static EventCallback<UIChangeEventArgs> CreateBinder<T>(
             this EventCallbackFactory factory,
             object receiver,
             Action<T> setter,
-            T existingValue) where T : Enum
+            T existingValue) where T : struct, Enum
         {
             return CreateBinderCore<T>(factory, receiver, setter, EnumConverter<T>.Convert);
         }
@@ -399,11 +618,27 @@ private static EventCallback<UIChangeEventArgs> CreateBinderCore<T>(
             this EventCallbackFactory factory,
             object receiver,
             Action<T> setter,
-            Func<object, T> converter)
+            BindConverter<T> converter)
         {
             Action<UIChangeEventArgs> callback = e =>
             {
-                setter(converter(e.Value));
+                T value = default;
+                var converted = false;
+                try
+                {
+                    converted = converter(e.Value, out value);
+                }
+                catch
+                {
+                }
+
+                // We only invoke the setter if the conversion didn't throw. This is valuable because it allows us to attempt
+                // to process invalid input but avoid dirtying the state of the component if can't be converted. Imagine if
+                // we assigned default(T) on failure - this would result in trouncing the user's typed in value.
+                if (converted)
+                {
+                    setter(value);
+                }
             };
             return factory.Create<UIChangeEventArgs>(receiver, callback);
         }