-2

How can I make this code better? It would be better if I could modify method GetItemsInfo() to send several types at one call.

using System.Collections.Generic;
using System.Linq;

public class SomeClass
{
  List<Item> itemBase;
...
   public IEnumerable<ItemInfo>  SomeMethod()
   {
      return itemBase.GetItemsInfo<GeneralItem>()
                .Concat(itemBase.GetItemsInfo<FirstTypeItem>())
                .Concat(itemBase.GetItemsInfo<SecondTypeItem>())
                .Concat(itemBase.GetItemsInfo<ThirdTypeItem>());
   }
}

where

public class ItemInfo
{...}
public class GeneralItem :ItemInfo
{...}
public class FirstTypeItem :ItemInfo
{...}
...
List<ItemInfo> items;
...
public IEnumerable<ItemInfo> GetItemsInfo<T>() where T: ItemInfo
        {
            return items.Where(item => item.GetType().Equals(typeof(T)));
        }

Thanks!

Update. Question updated by concrette realization of GetItemsInfo.

One more update. For now I wrote non-generic method

 public IEnumerable<ItemInfo> GetItemsInfo(params System.Type[] types) 
        {
            return items.Where(item => types.Any(type => type.Equals(item.GetType())));
        }

But It isn't looks pretty. Because in this array user can add any Type he wants even not child of ItemInfo. Maybe it's not a problem cause items is the collection of ItemInfo objects.

Chris Catignani
  • 5,040
  • 16
  • 42
  • 49
  • 1
    Well what is `GetItemsInfo`? Yes, you could do `GetItemsInfo()`. – DavidG Nov 11 '19 at 13:38
  • It would help if you explain to us *what* you try to achieve instead of *how* you want to achieve it – Fabjan Nov 11 '19 at 13:40
  • How does your `GetItemsInfo` implementation look? Do you want it to return all subtypes of `Item`? – Magnus Nov 11 '19 at 13:44
  • I have a collection of many different Item Infos. So I'm trying to get a number of item infos of certain types. And in every part of code there are different list of certain types `public IEnumerable GetItemsInfo() where T: ItemInfo { return items.Where(item => item.GetType().Equals(typeof(T))); }` – HedgehogNSK Nov 11 '19 at 13:49
  • 1
    This is primarily opinion-based question. From my POV, `GetItemsInfo` would look better with `return items.OfType();`. You can also pass an array of types to the method to match using a single loop instead of concatenating. – Yeldar Kurmangaliyev Nov 11 '19 at 13:58
  • 1
    I think this question would fit better on [codereview.se] since the OP wants to improve his code. –  Nov 11 '19 at 14:02
  • @Amy Code Review doesn't accept hypothetical/stub code though. – JAD Nov 11 '19 at 14:06

2 Answers2

1

Are you looking for the OfType method? Then I would think you'd need:

public IEnumerable<ItemInfo>  SomeMethod()
{
    return itemBase.OfType<GeneralItem>()
        .Concat(itemBase.OfType<FirstTypeItem>())
        .Concat(itemBase.OfType<SecondTypeItem>())
        .Concat(itemBase.OfType<ThirdTypeItem>());
}

Means you've reinvented the wheel... And you might even shorten it to:

public IEnumerable<ItemInfo>  SomeMethod()
{
    return itemBase.OfType<ItemInfo>();
}

As all the classes you're filtering share this common ancestor.


For an exact match on type, you could use this extention method inside a static class:

public static IEnumerable<T> MatchType<T>(this IEnumerable<object> data)
=> data.Where(i => i.GetType() == typeof(T)).Cast<T>();

Now you would use this code instead:

public IEnumerable<ItemInfo>  SomeMethod()
{
    return itemBase.MatchType<GeneralItem>()
        .Concat(itemBase.MatchType<FirstTypeItem>())
        .Concat(itemBase.MatchType<SecondTypeItem>())
        .Concat(itemBase.MatchType<ThirdTypeItem>());
}

Which is similar to your code, actually. A bit more generic, though. But you want a concat method that onle concats a select number of items, so you might want this method:

public static IEnumerable<object> ConcatSelect<T>(this IEnumerable<object> first, IEnumerable<object> second)
=> first.Concat(second.Where(i => i.GetType() == typeof(T)));

Again very generic so it's more reusable. This would result in this code:

public IEnumerable<ItemInfo>  SomeMethod()
{
    return new List<object>().
        .ConcatSelect<GeneralItem>(itemBase)
        .ConcatSelect<FirstTypeItem>(itemBase)
        .ConcatSelect<SecondTypeItem>(itemBase)
        .ConcatSelect<ThirdTypeItem>(itemBase)
        .Cast(ItemInfo);
}

Looks okay to me. Extremely generic, also.

Wim ten Brink
  • 25,901
  • 20
  • 83
  • 149
  • Thank you and to others who tell me about **OfType**. This classes have common parent. But I don't need all childs of **ItemInfo**. I need only _certain types_. And, as I said before, each time I suggest to use this method, there will be **different** collections of child types. – HedgehogNSK Nov 11 '19 at 14:35
  • @HedgehogNSK, I've changed my answer to suggest two alternative solutions. The ConcatSelect<> method might be what you need? – Wim ten Brink Nov 11 '19 at 16:59
  • 1
    not quite. I have been trying to found the solution which allows to me to pass more then 1 type as arguments to get list of ItemInfos which includes element of any of those types. Besides, I think that SomeMethod() of yours wouldn't work cause MatchType returns IEnumerable and the compiler won't let to concat IEnumerable of different types. I'm thinking I will use this signature `IEnumerable GetItemsInfo(params System.Type[] types) ` It's to pitty that it is without generics but it still works. Anyway, thanks for help. You gave me helpful information to think about. – HedgehogNSK Nov 12 '19 at 04:49
  • The ConcatSelect returns enumerations of objects, which are added to a List. It should be compatible. The trick is the Cast at the end, which turns the list into an IEnumerable. MatchType also does a cast back to T. – Wim ten Brink Nov 12 '19 at 05:23
0

Firstly, you can use OfType linq function instead of the equality check you are doing.

You can found more information here.

Now for your question, What you search is how to call a generic function with reflection. Their are already a lot of answers for this question.

You can found a good article here.

Ygalbel
  • 5,214
  • 1
  • 24
  • 32