Add LookupAs/TryLookupAs to complement Lookup/TryLookup - #688
Add LookupAs/TryLookupAs to complement Lookup/TryLookup#688Kenny Kerr (kennykerr) wants to merge 2 commits into
Conversation
|
/azp run |
Jon Wiswall (jonwis)
left a comment
There was a problem hiding this comment.
Thank you! (Also, cc Howard Kapustein (@DrusTheAxe))
|
|
||
| // TryLookupAs edge cases | ||
|
|
||
| REQUIRE_FALSE(m.TryLookupAs<bool>(L"INVALID").has_value()); |
There was a problem hiding this comment.
Does "nullptr" here represent the invalid value? Or should there be a test for m.TryLookupAs<Uri>(L"INVALID") here?
There was a problem hiding this comment.
TryLookupAs<bool> will return a std::optional<bool>, and we are verifying that it returns a valueless object.
In the second test, TryLookupAs<Uri> will return a Uri, and we are using the Uri == nullptr test to see if the Uri object is empty.
There was a problem hiding this comment.
How about a test that validates TryLookupAs<IInspectableDerivedThing>(L"VALUE THAT IS NOT IN THE MAP")?
There was a problem hiding this comment.
I preferred the proposal to add support for value types to IInspectable.as<> and IInspectable.try_as<> (treating them as unboxing), because that is useful in places other than map lookups. For example, converting XAML tags and navigation parameters.
// old and busted
auto bounds = unbox_value<Rect>(SelectedItem().as<FrameworkElement>().Tag());
auto userid = unbox_value<hstring>(navigationEventArgs.Parameter());
// new hotness
auto bounds = SelectedItem().as<FrameworkElement>().Tag().as<Rect>();
auto userid = navigationEventArgs.Parameter().as<hstring>();Even further, try_as could take a second parameter that is the fallback value. This is extremely handy when doing map lookups.
// with this proposal
auto optional_priority = settings.TryLookupAs<PriorityLevel>(L"priority");
auto priority = maybe_priority ? maybe_priority.value() : PriorityLevel::Normal;
// newer hotness
auto priority = settings.TryLookup(L"priority").try_as<PriorityLevel>(PriorityLevel::Normal);If we are hesitant to overload as and try_as in that way, we could call the new methods unbox_value, try_unbox_value, and unbox_value_or to match the free functions.
auto bounds = SelectedItem().as<FrameworkElement>().Tag().unbox_value<Rect>();
auto userid = navigationEventArgs.Parameter().unbox_value<hstring>();
auto priority = settings.TryLookup(L"priority").unbox_value_or<PriorityLevel>(PriorityLevel::Normal):|
|
||
| // TryLookupAs edge cases | ||
|
|
||
| REQUIRE_FALSE(m.TryLookupAs<bool>(L"INVALID").has_value()); |
There was a problem hiding this comment.
TryLookupAs<bool> will return a std::optional<bool>, and we are verifying that it returns a valueless object.
In the second test, TryLookupAs<Uri> will return a Uri, and we are using the Uri == nullptr test to see if the Uri object is empty.
FWIW, in addition to the Xaml examples Raymond listed, we have a new UIAutomation platform API (currently in insider preview SDK builds) that would also benefit from a uniform way of converting an For example; // today
auto elementResult = result.GetOperand(a).as<IUIAutomationElement>(); // this is a COM interface
auto textRangeResult = result.GetOperand(b).as<winrt::AutomationTextRange>(); // this is a WinRT runtimeclass
auto stringResult = winrt::unbox_value<winrt::hstring>(result.GetOperand(c));
// new hotness :)
auto elementResult = result.GetOperand(a).as<IUIAutomationElement>();
auto textRangeResult = result.GetOperand(b).as<winrt::AutomationTextRange>();
auto stringResult = result.GetOperand(c).as<winrt::hstring>();The consistency looks really appealing from a consumer's perspective, IMO. |
|
Yes, I also prefer overloading |
Fixes #427 by adding unboxing helpers for maps.