1

Currently I'm working with a lot of classes and lists of those classes. Previously I had static methods which would take a particular list of classes and return a subset or single item of the list.

However I thought it would be more convenient to have classes that inherit List and have the necessary methods there and no longer make them static, so they apply with whatever list of objects you are using.

I have been unable to find a simple way to convert a List to my class that inherits this, so I created a method in all of these collection classes to convert it for me.

Example:

public class Student 
{
    public int Id {get;set;}
    public string Name {get;set;}
}
public class Students : List<Student>
{
    public Student GetTopStudent()
    {
        return this.OrderByDescending(s => s.Grade).FirstOrDefault();
    }

    public Students GetPassingStudents()
    {
        return this.Where(s => s.Grade > 0.7).ToCollection();
    }

    public Students ToCollection(IEnumerable<Student> studentsList)
    {
        var students = new Students();
        foreach(var s in studentsList)
        {
            students.Add(s);
        }
        return students();
    ]
}

I have many other classes and lists of classes like this, but thats a pretty simple example. I'm finding some of my methods, like the "ToCollection()" method are virtually identical, between classes, with the exception of the return type and the type contained in the list.

So I've tried to create extensions and interfaces take care of these methods automatically.

Collection Class and Interfaces

public abstract class Collection<T> : List<T>
{
    // Needed tie in the extension method with the List.Add() Method
    public void Add(object item) => base.Add((T)item);
}

public interface IObjectWithId<T> 
    where T : IObjectWithId<T>
{
    int Id {get;set;}
}

public ICollectionItem<C> 
    where C : Collection<IcollectionItem<C>>
{
}

public ICollectionItemWithId<C,T> 
    where C : Collection<ICollectionItemWithId<C,T>>
    where T : IObjectWithId<T>
{
}

Extensions

public static List<T> Get<T>(this IEnumerable<IobjectWithId<T>> list, List<int> ids)
    where T : IObjectWithId<T>
{
    return list.Where(i => ids.Contains(i.Id))
        .Cast<T>();
        .ToList();
}

public static C Get<C, T>(this IEnumerable<IcollectionItemWithId<C, T>> list, List<int> ids)
    where C : Collection<ICollectionItemWithId<C, T>>, new()
    where T : IObjectWithId<T>
{
    return list.Where(i => ids.Contains(i.Id)).ToCollection();
}

public static C ToCollection<C, T>(this IEnumerable<ICollectionItemWithId<C, T>> list)
    where C : Collection<ICollectionItemWithId<C, T>>, new()
    where T : IObjectWithId<T>
{
    var collection = new C();
    foreach(var item in list)
    {
        collection.Add(item);
    }
    return collection;
}

public static C ToCollection<C>(this IEnumerable<ICollectionItem<C>> list)
where C : Collection<ICollectionItem<C>>, new()
{
    var collection = new C();
    foreach(var item in list)
    {
        collection.Add(item);
    }
    return collection;
}

I've been unable to get this code to work. Normally I get errors after building it, either there is no implicit reference conversion from the student class to the list of students or an error pertaining to boxing.

I'm concerned that my interfaces referencing eachother could be creating a lot of problems, however the reason I do this is so I don't have to specify the return type of a method, although perhaps that would be the easiest thing to do at this point...

Austin
  • 13
  • 3
  • 1
    Did you know that [it's not considered a good practice to inherit from `List`?](https://stackoverflow.com/questions/21692193/why-not-inherit-from-listt?rq=1) – Camilo Terevinto May 09 '18 at 22:32
  • I was not aware off this, but the comments on that page make a lot of sense. I appreciate you sharing it. Although one of the points that seemed to come up, was essentially is your "list" really just a class that also contains a list, or is it actually extending the list itself. For my implementation, my "lists" don't have any properties in them whatsoever, they only include additional methods to add functionality that is specific to a list of the classes they contain. So I wonder if this usage is a more acceptable form of an already questionable implementation? – Austin May 09 '18 at 23:26
  • From what you shared, the class is more like helper methods for business logic than an extension to a list. I'd consider dropping the inheritance there, and probably you don't even need a list property – Camilo Terevinto May 09 '18 at 23:29

1 Answers1

0

You are overcomplicating things. The root problem is your curiously self-referencing public interface IObjectWithId<T>. Since you don't use T anywhere in the interface itself this does not need to be generic.

Just write extension methods that work on IEnumerable<T> and return IEnumerable<T> with some simple constraints on T, e.g.

// This does not need to be generic in any way
public interface IObjectWithId
{
    int Id {get;set;}
}

public static IEnumerable<T> Get<T>(this IEnumerable<T> list, List<int> ids)
    where T : IObjectWithId
{
    return list.Where(i => ids.Contains(i.Id));
}

Let the caller chain .ToList() onto this if they want to. Sometimes they might only want only one (Single() or First()), or they might do another filter or group operation after it so forcing it back to a List<T> is inefficient.

Ian Mercer
  • 38,490
  • 8
  • 97
  • 133
  • I'm surprised the IObjectWithId was the only concern. To be honest I was a little insecure about all of the other self referencing interfaces/collection class. I'll give this a shot and see where it gets me. Thanks for the suggestion. – Austin May 09 '18 at 23:29
  • @Austin I think if you fix that many of the other problems go away including any need to inherit from `List` or to create another Collection class. – Ian Mercer May 09 '18 at 23:31