Is there a better way to get a child?

Question

I have written the following code to get ImageColorPicker child:

foreach (CustomTabItem customTabItem in SelectedWindowsTabControl.Items)
{
    TabItem ti = tabControl.ItemContainerGenerator.ContainerFromItem(customTabItem) as TabItem;
    Popup popup = (Helpers.FindVisualChild<Popup>(ti) as Popup);
    ImageColorPicker icp = (popup.Child as StackPanel).Children[0] as ImageColorPicker;

    ...
}

public class Helpers
{
    /// <summary>
    /// Return the first visual child of element by type.
    /// </summary>
    /// <typeparam name="T">The type of the Child</typeparam>
    /// <param name="obj">The parent Element</param>
    public static T FindVisualChild<T>(DependencyObject obj) where T : DependencyObject
    {
        for (int i = 0; i < VisualTreeHelper.GetChildrenCount(obj); i++)
        {
            DependencyObject child = VisualTreeHelper.GetChild(obj, i);
            if (child != null && child is T)
                return (T)child;
            else
            {
                T childOfChild = FindVisualChild<T>(child);
                if (childOfChild != null)
                    return childOfChild;
            }
        }
        return null;
    }
}

Here's the control template XAML of the TabItem (the relevant part):

<ControlTemplate TargetType="{x:Type local:CustomTabItem}">
    <Grid Height="26" Background="{TemplateBinding Background}">
        <Grid.ColumnDefinitions>
            <ColumnDefinition Width="Auto" />
            <ColumnDefinition Width="Auto" />
        </Grid.ColumnDefinitions>
        <ContentPresenter Margin="5,0" HorizontalAlignment="Left" VerticalAlignment="Center" ContentSource="Header">
        </ContentPresenter>
        <StackPanel Grid.Column="1" Height="16" Margin="0,0,1,0" HorizontalAlignment="Center" VerticalAlignment="Center" Orientation="Horizontal">
            <ToggleButton x:Name="Edit" Width="16" Content="&#xE104;" Style="{StaticResource CustomizedMetroTabItemToggleButton}" ToolTip="Edit" />
            <Popup HorizontalOffset="{Binding Width, ElementName=Edit}" IsOpen="{Binding IsChecked, Mode=TwoWay, ElementName=Edit}" Placement="Left" PlacementTarget="{Binding ElementName=Edit}" PopupAnimation="Slide" StaysOpen="False" VerticalOffset="{Binding ActualHeight, ElementName=Edit}">
                <StackPanel>
                    <local:ImageColorPicker x:Name="ColorPicker" Width="100" Height="100" HorizontalAlignment="Center" Source="Images/ColorWheel.png" />
                </StackPanel>
            </Popup>
        </StackPanel>
    </Grid>
</ControlTemplate>

Is there a better way to get the ImageColorPicker than what I've done? (getting the TabItem, then the Popup and then the ImageColorPicker, I am sure there's a shorter way)

Answer 1

I don't like a class that's just called Helpers - that's generally a code smell, something that ends up a big dumping ground for anything that doesn't quite fit anywhere else. Be more specific when naming things, perhaps VisualHierarchyHelper would be a better name?

I'm using a very similar method - the main difference is essentially the number ot return statements, and the childName parameter; I found this code on Stack Overflow a little while ago:

    /// <summary>
    /// Finds a Child of a given item in the visual tree. 
    /// </summary>
    /// <param name="parent">A direct parent of the queried item.</param>
    /// <typeparam name="T">The type of the queried item.</typeparam>
    /// <param name="childName">x:Name or Name of child. </param>
    /// <returns>The first parent item that matches the submitted type parameter. 
    /// If not matching item can be found, 
    /// a null parent is being returned.</returns>
    /// <remarks>
    /// https://stackoverflow.com/a/1759923/1188513
    /// </remarks>
    public static T FindChild<T>(this DependencyObject parent, string childName)
       where T : DependencyObject
    {
        if (parent == null) return null;

        T foundChild = null;

        var childrenCount = VisualTreeHelper.GetChildrenCount(parent);
        for (var i = 0; i < childrenCount; i++)
        {
            var child = VisualTreeHelper.GetChild(parent, i);
            var childType = child as T;
            if (childType == null)
            {
                foundChild = FindChild<T>(child, childName);
                if (foundChild != null) break;
            }
            else if (!string.IsNullOrEmpty(childName))
            {
                var frameworkElement = child as FrameworkElement;
                if (frameworkElement != null && frameworkElement.Name == childName)
                {
                    foundChild = (T)child;
                    break;
                }
            }
            else
            {
                foundChild = (T)child;
                break;
            }
        }

        return foundChild;
    }

Notice the guard clause preventing a NullReferenceException that your method would throw if obj was null. I think this is a pretty neat way of finding a child node in the visual tree.

That said, it might be personal preference, but I think the readability of your code could benefit from implicit typing (var), especially in cases like this where the type is already pretty obvious:

TabItem ti = tabControl.ItemContainerGenerator.ContainerFromItem(customTabItem) as TabItem;
Popup popup = (Helpers.FindVisualChild<Popup>(ti) as Popup);
ImageColorPicker icp = (popup.Child as StackPanel).Children[0] as ImageColorPicker;

Becomes:

var ti = tabControl.ItemContainerGenerator.ContainerFromItem(customTabItem) as TabItem;
var popup = (Helpers.FindVisualChild<Popup>(ti) as Popup);
var icp = (popup.Child as StackPanel).Children[0] as ImageColorPicker;

And here you would have to make sure popup isn't null before accessing its Child member, if you want to avoid that possible NullReferenceException.

Also, you're casting too much - T should be of the type you've specified, so the return type of FindChild<ImageColorPicker> is ImageColorPicker, casting it to ImageColorPicker is redundant.

---

Update

The ImageColorPicker child has a Popup parent, which has a StackPanel parent, which has a Grid parent, which has a TabItem parent.

You're not fully using the recursiveness of your function when you're getting the color picker. I'd believe you could get it like this:

var tab = tabControl.ItemContainerGenerator.ContainerFromItem(customTabItem) as TabItem;
var picker = VisualHierarchyHelper.FindChild<ImageColorPicker>(tab, "ColorPicker");

That should work, because the search is recursive; you don't need to get everything in-between.