App crash if Binding is null

[ChrisProlls]

I have a ListView binded to a list of MenuItem

    public class MenuItem
    {
        public string Icon { get; set; }
        public string Title { get; set; }
        public string Page { get; set; }
    }

public ObservableCollection<BeloteMenuItem> _menuItems = new ObservableCollection<BeloteMenuItem>(
            new[]
            {
                new BeloteMenuItem { Icon = "fa-home"  },
                new BeloteMenuItem { Icon = null }
            });

<ListView x:Name="MenuItemsListView"
              SeparatorVisibility="None"
              HasUnevenRows="true"
              ItemsSource="{Binding MenuItems}">
        <ListView.ItemTemplate>
                <DataTemplate>
                    <ViewCell>
                        <StackLayout Padding="15,10" HorizontalOptions="FillAndExpand">
                            <iconize:IconImage HeightRequest="20" Icon="{Binding Icon}" IconColor="Black" WidthRequest="20" />
                            <Label VerticalOptions="FillAndExpand" 
                                VerticalTextAlignment="Center" 
                                Text="{Binding Title}"
                                FontSize="17"/>
                        </StackLayout>
                    </ViewCell>
                </DataTemplate>
            </ListView.ItemTemplate>
</ListView>

In my example I volontary set one of my icon to null and the app immediately crashes, an the Xamarin.Forms error is not very convenient to debug.

[candidodmv]

Maybe your issue is related with this: #9

[tonholis]

+1 here

[tonholis]

I checked the IconImageRenderer which is causing a crash for me:

Iconize/src/Plugin.Iconize/Platform/Android/Renderers/IconImageRenderer.cs

Lines 77 to 101 in 6638c67

	private void UpdateImage()
	{
	var icon = Iconize.FindIconForKey(Image.Icon);
	if (icon is null)
	{
	#if USE_FASTRENDERERS
	SetImageResource(Android.Resource.Color.Transparent);
	#else
	Control.SetImageResource(Android.Resource.Color.Transparent);
	#endif
	return;
	}
	var iconSize = (Image.IconSize == IconImage.AutoSize ? Math.Max(Image.WidthRequest, Image.HeightRequest) : Image.IconSize);
	var drawable = new IconDrawable(Context, icon).Color(Image.IconColor.ToAndroid())
	.SizeDp((Int32)iconSize);
	#if USE_FASTRENDERERS
	SetScaleType(Image.IconSize > 0 ? ScaleType.Center : ScaleType.FitCenter);
	SetImageDrawable(drawable);
	#else
	Control.SetScaleType(Image.IconSize > 0 ? ScaleType.Center : ScaleType.FitCenter);
	Control.SetImageDrawable(drawable);
	#endif
	}

It already checks if an icon variable is null. So wouldn't be better if Iconize.FindIconForKey(iconKey) returns null instead of throwing an ArgumentNullException?

Iconize/src/Plugin.Iconize/Iconize.cs

Lines 126 to 135 in 6638c67

	public static IIcon FindIconForKey(String iconKey)
	{
	if (iconKey is null)
	throw new ArgumentNullException(nameof(iconKey));
	if (String.IsNullOrWhiteSpace(iconKey))
	throw new ArgumentException($"{nameof(iconKey)} cannot be empty", nameof(iconKey));
	return Modules?.FirstOrDefault(x => x.Keys.Contains(iconKey))?.GetIcon(iconKey) ?? throw new NullReferenceException($"No icon with the key: {iconKey}");
	}

Then it would SetImageResource(Android.Resource.Color.Transparent) and not cause a crash.
You could even write a log a "Icon not found" for debuging purposes...

[tonholis]

Ok...
I noticed that the version 3.4.0.100 does exactly what I suggested above. So I downgraded from 3.4.0.103 to 3.4.0.100 and it's working fine now.

[koystinen]

Same issue happens when navigating away from a page that has IconImage that has Icon set via binding.
And downgrade to 3.4.0.100 has resolved the issue.

[koystinen]

@jsmarcus looks like the change was introduced by this commit: 6638c6735fca0d79c3a25bb465ab1297872ec94e

[StargazerNC]

I'm with @tonholis . That is a preferable behavior. Nevertheless, @jsmarcus is there any special rationale behind the change?

[CZEMacLeod]

I noticed that the return remark for the function actually says it will return null if the icon is not found.
I can understand on a public function, doing parameter checking, but perhaps it should be split into a TryGetIcon type function that doesn't throw and is used 'internally' by the renderers...

Iconize/src/Plugin.Iconize/Iconize.cs

Line 125 in 6638c67

/// <returns>The icon, or null if no icon matches the key</returns>