Simplifying for loops that includes conditions with better linq queries

Question

The code below selects all items that are locked. How can I optimise it?
ShopItemData doesn't contain a field for locked/unlocked.

_allItems is a ShopItemData[]

_codaCommonShopItems, _codaRareShopItems, and _codaEpicShopItems are CodaShopItem[]s.

public class CodaShopItem : MonoBehaviour
    {
        private Button _itemButton;
        private bool _inUse;
        private int _itemId;
        private CodaShopScreen.ItemRarity _itemRarity;

        public int ItemId => _itemId;
        public bool Unlocked;
        public bool InUse => _inUse;
        public CodaShopScreen.ItemRarity ItemRarity => _itemRarity;
}

public class ShopItemData : ScriptableObject
    {
        public int ItemId;
        public Sprite ItemSprite;
        public CodaShopScreen.ItemRarity ItemRarity;
        public GameObject InGamePrefab;
        public GameObject PreviewPrefab;
    }


public IEnumerable<ShopItemData> GetLockedItems()
        {
            var lockedCommonItems = _codaCommonShopItems.Where(item => !item.Unlocked).ToArray();
            var lockedRareItems = _codaRareShopItems.Where(item => !item.Unlocked).ToArray();
            var lockedEpicItems = _codaEpicShopItems.Where(item => !item.Unlocked).ToArray();
            var allLockedItems = new List<ShopItemData>();
            var allItemDatas = GetAllItems().ToArray();
            
            for (var i = 0; i < lockedCommonItems.Length; i++)
            {
                var lockedItemData = allItemDatas.First(item => item.ItemId == lockedCommonItems[i].ItemId && item.ItemRarity == lockedCommonItems[i].ItemRarity);
                allLockedItems.Add(lockedItemData);
            }
            
            for (var i = 0; i < lockedRareItems.Length; i++)
            {
                var lockedItemData = allItemDatas.First(item => item.ItemId == lockedRareItems[i].ItemId && item.ItemRarity == lockedRareItems[i].ItemRarity);
                allLockedItems.Add(lockedItemData);
            }
            
            for (var i = 0; i < lockedEpicItems.Length; i++)
            {
                var lockedItemData = allItemDatas.First(item => item.ItemId == lockedEpicItems[i].ItemId && item.ItemRarity == lockedEpicItems[i].ItemRarity);
                allLockedItems.Add(lockedItemData);
            }

            return allLockedItems;
        }

public IEnumerable<ShopItemData> GetAllItems()
        {
            return _allItems;
        }

And this is how shop items are created

private void CreateCommonShopItems()
        {
            var commonItems = Resources.LoadAll<ShopItemData>(ShopItemDataPath).Where(item => item.ItemRarity == ItemRarity.Common);
            var commonCategory = _itemCategories.First(item => item.ItemRarity == ItemRarity.Common);
            commonCategory.Create(ref _codaCommonShopItems, commonItems.ToArray());
        }

basically I am getting the item datas and passing them into another function which turns them into _codaCommonShopItem and fill their properties

_codaCommonShopItem[i].Init(items[i].ItemId, items[i].ItemRarity, items[i].ItemSprite);

Answer 1

In short terms, you can do this :

public IEnumerable<ShopItemData> GetLockedItems()
{
    var allLockedItems = new List<ShopItemData>();
    
    allLockedItems.AddRange(_codaCommonShopItems.Where(item => !item.Unlocked));
    allLockedItems.AddRange(_codaRareShopItems.Where(item => !item.Unlocked));
    allLockedItems.AddRange(_codaEpicShopItems.Where(item => !item.Unlocked));
    
    var result = _allItems
                    .Join(allLockedItems, 
                    items  => items.ItemId, 
                    locked => locked.ItemId,
                    (items, locked) => new { AllItems = items, LockedItems = locked })
                    .Where(x => x.AllItems.ItemId == x.LockedItems.ItemId && x.AllItems.ItemRarity == x.LockedItems.ItemRarity)
                    .Select(x=> new ShopItemData
                    {
                        ItemId = AllItems.ItemId,                       
                        ItemSprite = AllItems.ItemSprite,
                        ItemRarity = AllItems.ItemRarity, 
                        InGamePrefab = AllItems.InGamePrefab, 
                        PreviewPrefab = AllItems.PreviewPrefab
                    });

    return result;
} 

So, you add all unlocked items in the list, then join the new list with the _allItems on the same condition you use.

What I suggest, is to make a child class that would flatten and combine both objects properties. something like this :

private class JoinedShopItemData
{
    public int ItemId { get; set; }

    public bool Unlocked { get; set; }

    public bool InUse { get; set; }

    public CodaShopScreen.ItemRarity ItemRarity { get; set; }

    public Sprite ItemSprite { get; set; }

    public GameObject InGamePrefab { get; set; }

    public GameObject PreviewPrefab { get; set; }
}

then, you can initialize this object in your constructor, and prepare it to be used as filtered source of _allItems for the current class.

Something like :

private IEnumerable<JoinedShopItemData> GetJoinedShopItemData(IEnumerable<CodaShopItem> source)
{
    foreach(var coda in source)
    {
        foreach(var item in _allItems)
        {
            if(item.ItemId == coda.ItemId && item.ItemRarity == coda.ItemRarity)
            {
                yield return new JoinedShopItemData
                {
                    ItemId          = AllItems.ItemId,                      
                    ItemSprite      = AllItems.ItemSprite,
                    ItemRarity      = AllItems.ItemRarity, 
                    InGamePrefab    = AllItems.InGamePrefab, 
                    PreviewPrefab   = AllItems.PreviewPrefab, 
                    Unlocked        = LockedItems.Unlocked, 
                    InUse           = LockedItems.InUse     
                }; 
            }
        }
    }
}

or use linq (like the first example) Now, assuming you'll have a global List<JoinedShopItemData> named _joinedShopItemData that will hold GetJoinedShopItemData results.

In this case, your GetLockedItems method would be :

public IEnumerable<ShopItemData> GetLockedItems()
{
    return _joinedShopItemData
    .Where(item => !item.Unlocked && (item.ItemRarity == ItemRarity.Common || item.ItemRarity == ItemRarity.Rare || item.ItemRarity == ItemRarity.Epic))
    .Select(item => new ShopItemData 
    {
        ItemId = item.ItemId,                       
        ItemSprite = item.ItemSprite,
        ItemRarity = item.ItemRarity, 
        InGamePrefab = item.InGamePrefab, 
        PreviewPrefab = item.PreviewPrefab
    }).ToArray();
} 

This will elemnate the need of creating a separate collection for each property condition (like _codaCommonShopItems, _codaRareShopItems ..etc.). As you will always have a joined object which you can access easily.