TreeView selection APIs

[kaiguo]

Fix #197
Fix #124
Fix #386

We already have SelectedNodes to get selected TreeViewNodes in multi-selection mode.

Added

• SelectedNode: get the selected TreeViewNode in single selection mode
• SelectedItem: get the selected item in single selection mode
• SelectedItems: get selected items in multi-selection mode

SelectedNode(s) always returns TreeViewNode.
SelectedItem(s) returns the ItemsSource type object when using databinding, or TreeViewNode when not using databinding.

[ranjeshj]

nit: avoid multiple points of return if possible. I think that makes it easier to read and debug.

winrt::DependencyObject value = nullptr;

if(node)
{
value = IsContentMode() ? ContainerFromItem(node.Content()): ContainerFromItem(node);
}

return value;

same for methods below as well.

[jevansaks]

I think multiple / early returns are fine as long as the function is short enough. I think that the presented alternative is slightly harder to read (for me). But we shouldn't have coding style discussions in CR -- @chrisglein I believe this one has been discussed before but I don't see it captured in the coding style guide.

[chrisglein]

There's not a hard and fast rule on this one. Avoiding early returns altogether leads to the "arrow" pattern where you're overindented and it can be hard to determine what's validation versus real conditional. Doing too many early returns can create problems with object lifetimes/initialization and sometimes make the logic flow hard to read if the early returns have too many different outcomes.

Most of the universal pushback on early returns came from clean up patterns. Embracing RAII, smart pointers, and scope helpers have alleviated that and made error goto macros obsolete. So early returns aren't a bad word as they once were.

We have talked about this but haven't captured it in a style guide because we don't have hard and fast rules. The guideline I would push is to prefer early returns to extra condition hierarchy and indentation because an early return generally reduces the mental load reading the function. You no longer have to think about the condition that returned, as opposed to tracking the condition scopes and thinking about how that could flow.

tl;dr: I need to bring more of our code style guide over to the GitHub docs, and add some commentary on this topic.

[ranjeshj]

For smaller functions I agree that this is less of a concern. One thing I like is that I don't have to think about the negative cases intermixed with the happy path. All the if, then's are the happy path which is simpler to parse in my brain for me. Either have a specific else to modify the state or the default is just passed along in which case I don't need to worry about the else blocks. Another is that I can put a breakpoint at the end of the method and see what the returned value is. If there are multiple returns, then you cannot do that. I'm sure there are pro's and con's on this - see this thread - https://softwareengineering.stackexchange.com/questions/18454/should-i-return-from-a-function-early-or-use-an-if-statement . Again, for small functions, I don't think it matters, for larger ones, I would lean towards single exit.

[jevansaks]

@kaiguo can you also kick off the API spec review for this? (or have you already done that?)

[kaiguo]

@kaiguo Kai Guo FTE can you also kick off the API spec review for this? (or have you already done that?)

Not yet, creating the spec doc now...

[kaiguo]

@kaiguo Kai Guo FTE can you also kick off the API spec review for this? (or have you already done that?)

@jevansaks spec PR here, microsoft/microsoft-ui-xaml-specs#31

reset

[msft-github-bot]

🎉Microsoft.UI.Xaml v2.2.190731001-prerelease has been released which incorporates this pull request.:tada:

Handy links:

• Release Notes
• Official Nuget Release