Search code examples
c#winformsclass-design

Where to initialize collection of another class


Where's the best place to initialize a collection of objects? I started working on an older project that was previously very sensitive about making database calls... so we'd have something like this:

public class Car
{
    public int ID { get; set; }
    public string Make { get; set; }

    public Car(int id) {}

    public void AddCar() {}
    public void EditCar() {}
    public void PopulateAllCarInfo() {}
}

public class CarCollection : IEnumerable
{
    public int this[int index] { get return CarIDs[index - 1] }

    public CarCollection(string database)() // Populates CarIDs

    public List<int> CarIDs;

    public Car GetCarByID(int id){
        Car c = new Car(id);
        c.PopulateAllCarInfo();
        return c;    
    }
}

So in order to retrieve a full collection, I need to do this

CarCollection cars = new CarCollection("database");
List<Car> carDetails = new List<Car>();
foreach (int carID in cars)
{
    Car c = new Car(carID);
    c.PopulateAllCarInfo();
    carDetails.Add(c);
}

I'm a newbie to the team and I'll be refactoring this to get to know the codebase. What's the best way to populate a collection of Cars? Is the separate class overkill?

I was thinking of trying to create a new CarCollection that does...

public CarCollection
{
    // This method would populate the info for all cars
    public List<Car> RetrieveCars("database") {}

    // Leave this so I can still retrieve only Car data for single cars if I want
    public List<int> ListCarIDs() {}
}

And move the methods that involve accessing only one car to Car

public Car
{
    public Car GetCarByID(int id) {} // Populate Car
}

Question: is the CarCollection class overkill? Where do you put your methods for retrieve collections? (Note we're not using MVC or any other pattern)

I did find this but it didn't have any suggestions for how to retrieve full collections: https://softwareengineering.stackexchange.com/questions/196125/is-it-a-good-practice-to-create-a-classcollection-of-another-class


Solution

  • What's the best way to populate a collection of Cars?

    Classes should not populate their own data from a data source - at worst that ties your class to a particular data source, at best is adds a weak dependency to some data source.

    Generally a class such as a repository is responsible for loading data from a source, and using that data to create objects, using either the object's constructor or public properties.

    So in your case, a good design would be to create a CarRepository that can create a collection of Cars by loading data from a source, and save any changes back to the source.

    Is the CarCollection class overkill?

    Yes - you should be able to just use List<Car> as a concrete type and IEnumerable<Car> when you just need to iterate the collection (not add to it). You certainly shouldn't implement the non-generic IEnumerable as you lose type safety when enumerating the collection.