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: https://github.com/jsmarcus/Iconize/blob/6638c6735fca0d79c3a25bb465ab1297872ec94e/src/Plugin.Iconize/Platform/Android/Renderers/IconImageRenderer.cs#L77-L101

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? https://github.com/jsmarcus/Iconize/blob/6638c6735fca0d79c3a25bb465ab1297872ec94e/src/Plugin.Iconize/Iconize.cs#L126-L135

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...

https://github.com/jsmarcus/Iconize/blob/6638c6735fca0d79c3a25bb465ab1297872ec94e/src/Plugin.Iconize/Iconize.cs#L125