Is there any way that I can simplify the use of multiple "if's" inside one another

Question

Here's an example of what I mean. The code works but I wonder if there's another way that I can implement the same functionality:

        if (App.ShowIcons)
        {
            if (App.devIsIOS)
            {
                this.Children.Add(FR1, 0, 0);
                this.Children.Add(GT, 1, 0);
                this.Children.Add(FR2, 2, 0);
            }
            else
            {
                this.Children.Add(FR1, 0, 0);
                this.Children.Add(GT, 1, 0);
                this.Children.Add(CE, 2, 0);
            }
        }
        else
        {
            if (App.devIsIOS)
            {
                this.Children.Add(GT, 0, 0);
                this.Children.Add(FR2, 1, 0);
            }
            else
            {
                this.Children.Add(GT, 0, 0);
                this.Children.Add(CE, 1, 0);
            }
        }

Update: Here is an issue that I get when trying to implement one of the answers:

There are several ways to write this. There is also nothing particularly wrong with the one you chose. You could also use a data-based approach, e.g. maintain a master table with attributes for each flag and then filter them to get the right list. Or you could have no logic, and instead use resource files, and ship different ones for iOS.. That sort of thing. There's endless options. It depends completely on what makes the logic as obvious as possible and easy to maintain.. — John Wu, Jun 20 '20 at 06:51
What is the type of `Children`? I want to know what `Add` does, and what the three arguments mean. — Sweeper, Jun 20 '20 at 07:30

score 4 · Answer 1 · answered Jun 20 '20 at 07:37

One possible way to reduce nesting is by using tuples and switch

switch (ShowIcons, devIsIOS)
{
    case (true, true):
        Children.Add(FR1, 0, 0);
        Children.Add(GT, 1, 0);
        Children.Add(FR2, 2, 0);
        break;
    case (true, false):
        Children.Add(FR1, 0, 0);
        Children.Add(GT, 1, 0);
        Children.Add(CE, 2, 0);
        break;
    case (false, true):
        Children.Add(GT, 0, 0);
        Children.Add(FR2, 1, 0);
        break;
    case (false, false):
        Children.Add(CE, 2, 0);
        Children.Add(GT, 0, 0);
        Children.Add(CE, 1, 0);
        break;
    default:
        throw new Exception("");
}

if this logic is not being used too much in application then its OK to use this. But if this logic is dynamic and subjected to change in future then try to write your code in object oriented way and handle logic using polymorphic instead of branching (if-else) as demonstrated in this blog.

Can you also document a solution using the polymorphic way? Then I'm happy to accept. Thanks — Alan2, Jun 20 '20 at 09:05
@Alan2 I cannot drain classes based on information in question, here is an example of how you can convert branching into classes [link](https://stackoverflow.com/a/519537/1735196) — Mujahid Daud Khan, Jun 21 '20 at 11:56

score 2 · Answer 2 · answered Jun 20 '20 at 07:51

This is a hard question to answer, there are a lot of ways to implement stuff like this and it then becomes a matter of personal preference.

That said this logic can be simplified and implemented without the use of nested if/else clauses, which by the title I hope is what you're looking for:

var index = 0;
if (App.ShowIcons)
    children.Add(FR1, index++, 0);
children.Add(GT, index++, 0);
children.Add(App.devIsIOS ? FR2 : CE, index, 0);

Pavel Anikhouski · Accepted Answer · 2020-06-30T10:38:13.760

2

Removing repetitive call from nested if statements can make the entire expression shorter and cleaner

if (App.ShowIcons)
{
    Children.Add(FR1, 0, 0);
    Children.Add(GT, 1, 0);
    Children.Add(App.devIsIOS ? FR2 : CE, 2, 0);
}
else
{
    Children.Add(GT, 0, 0);
    Children.Add(App.devIsIOS ? FR2 : CE, 1, 0);
}

According to comments, if FR2 and CE values have different types, you should cast them to common base type, like View in your case Children.Add(App.devIsIOS ? (View)FR2 : CE, 1, 0);

edited Jun 30 '20 at 10:38

answered Jun 20 '20 at 09:05

Pavel Anikhouski

21,776
12
51
66

Hello Pavel, I like your solution the best but encountered a problem. When implemented I get this message a message in the IDE as the type of FR2 and CE is different. I have put this message for view in the question. Do you have any ideas how to solve this? – Alan2 Jun 21 '20 at 07:33
What is the type of both `FR2` , `CE` and `Children` collection? Your screen in question doesn't help at all. Adding a cast to the base type can solve it – Pavel Anikhouski Jun 21 '20 at 09:06
FR2 is a Frame, CE is an Entry. Sorry for not providing all the information. Do you know if there is any base type those both share and if so, how can I do a cast like that? – Alan2 Jun 21 '20 at 11:22
1

@Alan2 You can try yo use base [`View`](https://learn.microsoft.com/en-us/dotnet/api/xamarin.forms.view?view=xamarin-forms) class, something like `Children.Add(App.devIsIOS ? (View)FR2 : CE, 1, 0);` – Pavel Anikhouski Jun 21 '20 at 14:08

Eatos · Answer 4 · 2020-06-20T11:44:28.117

I like Mujahid Daud Khan answer for using C# 8.0 patterns. For the polymorphic way I thing the article is TLDR though :P

At the basic level we can do somethink like this:

using System;
using System.Collections.Generic;

namespace ConsoleApp4
{
    class Program
    {
        static void Main()
        {
            var builder = new IconSelectorBuilder();

            Console.WriteLine($"{nameof(Device.DevIsIOS)}, false");
            foreach (var item in builder.Build(Device.DevIsIOS, false).GetIcons())
            {
                Console.WriteLine(item);
            }

            Console.WriteLine($"{nameof(Device.DevIsIOS)}, true");
            foreach (var item in builder.Build(Device.DevIsIOS, true).GetIcons())
            {
                Console.WriteLine(item);
            }

            //Console.WriteLine($"{nameof(Device.Other)}, false");
            //foreach (var item in builder.Build(Device.Other, false).GetIcons())
            //{
            //    Console.WriteLine(item);
            //}

            //Console.WriteLine($"{nameof(Device.Other)}, true");
            //foreach (var item in builder.Build(Device.Other, true).GetIcons())
            //{
            //    Console.WriteLine(item);
            //}
        }
    }

    public enum Device
    {
        DevIsIOS,
        Other
    }

    public class IconSelectorBuilder
    {
        public IIconSelector Build(Device device, bool showIcons)
        {
            return device switch
            {
                Device.DevIsIOS => new DevIsIOSIconSelector(showIcons),
                _ => new OtherIconSelector(showIcons),
            };
        }
    }

    public interface IIconSelector
    {
        public bool ShowIcons { get; set; }

        IEnumerable<Icon> GetIcons();
    }

    public abstract class IconSelectorBase : IIconSelector
    {
        public bool ShowIcons { get; set; }

        protected IconSelectorBase(bool showIcons)
        {
            ShowIcons = showIcons;
        }

        protected abstract IEnumerable<Icon> GetHideIcons();

        protected abstract IEnumerable<Icon> GetShowIcons();

        public IEnumerable<Icon> GetIcons() => ShowIcons ? GetShowIcons() : GetHideIcons();
    }


    public sealed class DevIsIOSIconSelector : IconSelectorBase
    {
        public DevIsIOSIconSelector(bool showIcons) : base(showIcons)
        {
        }

        protected override IEnumerable<Icon> GetHideIcons()
        {
            yield return new Icon
            {
                IconType = IconType.GT,
                MyProperty1 = 0,
                MyProperty2 = 0,
            };

            yield return new Icon
            {
                IconType = IconType.FR2,
                MyProperty1 = 1,
                MyProperty2 = 0,
            };
        }

        protected override IEnumerable<Icon> GetShowIcons()
        {
            yield return new Icon
            {
                IconType = IconType.FR1,
                MyProperty1 = 0,
                MyProperty2 = 0,
            };

            yield return new Icon
            {
                IconType = IconType.GT,
                MyProperty1 = 1,
                MyProperty2 = 0,
            };

            yield return new Icon
            {
                IconType = IconType.FR2,
                MyProperty1 = 2,
                MyProperty2 = 0,
            };
        }
    }

    public sealed class OtherIconSelector : IconSelectorBase
    {
        public OtherIconSelector(bool showIcons) : base(showIcons)
        {
        }

        protected override IEnumerable<Icon> GetHideIcons()
        {
            // TODO
            throw new NotImplementedException();
        }

        protected override IEnumerable<Icon> GetShowIcons()
        {
            // TODO
            throw new NotImplementedException();
        }
    }

    public class Icon
    {
        public IconType IconType { get; set; }

        public int MyProperty1 { get; set; }

        public int MyProperty2 { get; set; }

        public override string ToString() => $"{IconType}, {MyProperty1}, {MyProperty2}";
    }

    public enum IconType
    {
        FR1,
        GT,
        FR2,
        CE
    }
}

The idea is to create IconSelectorBuilder and provide parameters like App.devIsIOS/whatever, App.ShowIcons/whatever (I assumed bool). Then we construct a proper class which which inherits from IconSelectorBase which provides a logic for bool parameter (App.ShowIcons/'whatever'). Then the proper class will return proper Icons.

Output:

DevIsIOS, false
GT, 0, 0
FR2, 1, 0
DevIsIOS, true
FR1, 0, 0
GT, 1, 0
FR2, 2, 0

Is there any way that I can simplify the use of multiple "if's" inside one another

4 Answers4