Skip to content

Commit

Permalink
Remove ValueType support and reflection piece from TaskResultConverter
Browse files Browse the repository at this point in the history
Update description to explicitly call out returning null instead of default if no task or task result not ready.
  • Loading branch information
michael-hawker committed Oct 20, 2022
1 parent 1398210 commit e3fe539
Show file tree
Hide file tree
Showing 2 changed files with 12 additions and 15 deletions.
16 changes: 3 additions & 13 deletions Microsoft.Toolkit.Uwp.UI/Converters/TaskResultConverter.cs
Original file line number Diff line number Diff line change
Expand Up @@ -14,31 +14,21 @@ namespace Microsoft.Toolkit.Uwp.UI.Converters
/// This is needed because accessing <see cref="Task{TResult}.Result"/> when the task has not
/// completed yet will block the current thread and might cause a deadlock (eg. if the task was
/// scheduled on the same synchronization context where the result is being retrieved from).
/// The methods in this converter will safely return <see langword="default"/> if the input
/// The methods in this converter will safely return <see langword="null"/> if the input
/// task is not set yet, still running, has faulted, or has been canceled.
/// </summary>
/// <seealso href="https://learn.microsoft.com/dotnet/csharp/language-reference/builtin-types/default-values">Default values of C# types</seealso>
public sealed class TaskResultConverter : IValueConverter
{
/// <inheritdoc/>
public object Convert(object value, Type targetType, object parameter, string language)
{
//// Check if we need to return a specific type, which only matters for value types where the default won't be null.
var hasValueTypeTarget = targetType is not null && targetType.IsValueType;
//// If we have a value type, then calculate it's default value to return, as we probably need it unless the task is completed.
var defaultValue = hasValueTypeTarget ? Activator.CreateInstance(targetType) : null;

if (value is Task task)
{
// If we have a task, check if we have a result now, otherwise the non-generic version of this
// function always returns null, so we want to use whatever we actually want the default value to be
// for our target type (in case it's a value type).
return task.GetResultOrDefault() ?? defaultValue;
return task.GetResultOrDefault();
}
else if (value is null)
{
// If we have a value type, return that value, otherwise this will be null.
return defaultValue;
return null;
}

// Otherwise, we'll just pass through whatever value/result was given to us.
Expand Down
11 changes: 9 additions & 2 deletions UnitTests/UnitTests.UWP/Converters/Test_TaskResultConverter.cs
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ public class Test_TaskResultConverter
{
[TestCategory("Converters")]
[UITestMethod]
[Ignore] // Ignore this value type test. Behavior will return null currently and not default.
public void Test_TaskResultConverter_Instance_Int32()
{
TaskResultConverter converter = new();
Expand Down Expand Up @@ -92,9 +93,15 @@ public void Test_TaskResultConverter_Instance_NullObject()

Assert.AreEqual(null, converter.Convert(null, null, null, null));

Assert.AreEqual(0, (int)converter.Convert(null, typeof(int), null, null));
// TODO: Think there may still be a problem for value types in x:Bind expressions, represented by these tests here,
// but was going to be too big a change for 7.1.3, will have to get more feedback and evaluate later.
/*Assert.AreEqual(0, (int)converter.Convert(null, typeof(int), null, null));
Assert.AreEqual(false, (bool)converter.Convert(null, typeof(bool), null, null));
Assert.AreEqual(false, (bool)converter.Convert(null, typeof(bool), null, null));*/

Assert.AreEqual(null, converter.Convert(null, typeof(int), null, null));

Assert.AreEqual(null, converter.Convert(null, typeof(bool), null, null));

Assert.AreEqual(null, (int?)converter.Convert(null, typeof(int?), null, null));

Expand Down

0 comments on commit e3fe539

Please sign in to comment.