How to Refactor this big if condition-CodePudding

How to refactoring this big if condition?

if (Customanger.singleton.weapon[1].activeSelf || Customanger.singleton.weapon[2].activeSelf || Customanger.singleton.weapon[3].activeSelf
               || Customanger.singleton.weapon[4].activeSelf || Customanger.singleton.weapon[5].activeSelf || Customanger.singleton.weapon[8].activeSelf || Customanger.singleton.weapon[10].activeSelf
               || Customanger.singleton.weapon[12].activeSelf || Customanger.singleton.weapon[13].activeSelf || Customanger.singleton.weapon[14].activeSelf || Customanger.singleton.weapon[15].activeSelf || Customanger.singleton.weapon[16].activeSelf
               || Customanger.singleton.weapon[17].activeSelf
               ) {
dosomething()
}

CodePudding user response：

You can try .Any(),

Determines whether any element of a sequence exists or satisfies a condition.

//Here I considered length of Weapon array/list is 18
if (Customanger.singleton.weapon.Any(x => x.activeSelf))
{
    dosomething();
}

CodePudding user response：

I don't know which types you are using. What I did is check which pattern is recurrent. I see that you always access the weapon list/array. Normally indexers can be iterated.

Some assumes are made because of the incompleteness of information.

You could write a loop for it:

private bool CheckActiveSelf(List<WeaponThing> weapons)
{
    // iterate each weapon
    foreach(var weapon in weapons)
        // when activeSelf, return true
        if(weapon.activeSelf)
            return true;
   
    return false;
}


// instead of passing the Customanger, you should pass the deepest
// level. So when the weapon system is used elsewhere, you can still
// use the method.
if(CheckActiveSelf(Customanger.singleton.weapon))
{
    DoSomething();
}

CodePudding user response：

There are multiple ways to do it, you can include or exclude items from your collection and than check for the "activeSelf" flag ex:

List<int> excludedList = new List<int>() { 1, 3, 5, 8, 9, 10 };
if (Customanger.singleton.weapon.Except(excludedList).Any(x => x.activeSelf))
    dosomething();
    ```

CodePudding user response：

You could do something like that:

var weaponIds = new List<int>() { 1, 2, 3, 4, 5, 8, 10, 12, 13, 14, 15, 16, 17 };

var weapons = weaponIds.Select(x => Customanger.singleton.weapon[x]).ToList();

if (weapons.Where(x => x.activeSelf).Any())
{
    DoSomething();
}

You could probably do better than that by splitting your weapon arrays into categories maybe? I can't help you more with the code you provided.

CodePudding user response：

whatever this check means, in OOP world it should be implemented as instance method (or property) with meaningful name in Customanger class:

class Customanger
{
    private int[] activeweapons = { 1, 2, 3, 4, 5, 8, 10, 12, 13, 14, 15, 16, 17 };
    public bool HasActiveWeapon => activeweapons.Any(x => weapon[x].activeSelf);
}

then condition will be reduced to:

if (Customanger.singleton.HasActiveWeapon) { \*do smth*\ }