locked
Correct usage of returned data type in Repository Design Pattern RRS feed

  • Question

  • User206383436 posted

    I am developing my first ASP.NET MVC 5 application using the Repository Pattern and I wonder if I am doing things correctly. So, for this reason, I am posting part of my code here in order to get feedback from you and redirect my design if needed. I started by coding an Interface with the basic methods/operations that will be used to maintain the DB.

    public interface IRepository<T>
    {
        void Insert(T entity);
        void Delete(Int32 id);
        void Update(T entity);
        T GetById(Int32 id);
        IEnumerable<T> GetAll();
    }
    

    Next, I coded a basic Brand class with properties only which represents a table in the DB as follows:

    public class Brand
    {
        public Int32 Id { get; set; }
        public String Code { get; set; }
        public String Name { get; set; }
    }

    Finally, I created the BrandRepository class that implements the IRepository<T> interface. For simplicity, I am only including the method I interested in (GetAll()):

    public class BrandRepository : IRepository<Brand>
    {
        public IEnumerable<Brand> GetAll()
        {
            // Here I define:
            // * A query that will SELECT all the records from the Brands table.
            // * A connection to the database.
    
            // Data will be saved in a List.
            List<Brand> brands = new List<Brand>();
            try
            {
                // ... Open DB connection ...
                // ... Execute command to perform the query and get the result in an ADO.NET DataReader (I am not using Entity Framework)
    
                loop through DataReader called reader
                {
                    Brand brand = new Brand();
                    brand.Id = reader.GetInt32("id");
                    brand.Code = reader.GetString("code");
                    brand.Name = reader.GetString("name");
                    brands.Add(brand);
                    reader.Read();
                }
            }
            catch (Exception exception)
            {
                // Catch any error/exception.
            }
            finally
            {
                // Close DB connection.
            }
    
            // Return List.
            return brands;
        }
    }

    As you can see, the GetAll() method returns data of type IEnumerable<Brand> and I am using a List<Brand> explicitly to return it. I suspect that this is not the best approach because, what is the purpose of setting the return type as a general IEnumerable<Brand> and returning a specific List<Brand> type?

    I will very much appreciate your feedback.

    Respectfully,
    Jorge Maldonado

    Thursday, February 14, 2019 6:11 PM

Answers

  • User475983607 posted

    Is it correct to use a List<T> and return it?

    Sure because List<T> inherits from IEnumerable<T>.  Essentially List<T> can do everything IEnumerable<T> can do.

    Isn´t is the same if I set the return type as List<Brand> instead of IEnumerable<Brand>?

    You are currently returning List<T>.  If you are asking about the method return type you can change the return type to List<T> but that can be restrictive in your repository design.  

    What is the advantage of setting the return type as IEnumerable<Brand> in the method?

    You can return any type that implements IEnumerable<T>.  

    • Marked as answer by Anonymous Thursday, October 7, 2021 12:00 AM
    Thursday, February 14, 2019 8:04 PM

All replies

  • User475983607 posted

    I suspect that this is not the best approach because, what is the purpose of setting the return type as a general IEnumerable<Brand> and returning a specific List<Brand> type?

    List<T> inherits from the IEnumerable<T>.

    https://docs.microsoft.com/en-us/dotnet/api/system.collections.generic.list-1?view=netframework-4.7.2

    The principle is exactly the same as implementing the generic repository pattern. Writing to an interface.

    Thursday, February 14, 2019 6:29 PM
  • User206383436 posted

    Thanks for your reply.
    I know that List<T> inherits from IEnumerable<T>.
    Is it correct to use a List<T> and return it?
    Isn´t is the same if I set the return type as List<Brand> instead of IEnumerable<Brand>?
    What is the advantage of setting the return type as IEnumerable<Brand> in the method?

    Best regards,
    Jorge Maldonado

    Thursday, February 14, 2019 7:29 PM
  • User753101303 posted

    Hi,

    The interface is basically a "contract" with no implementation so it will be ALWAYS something more specific than IEnumerable<Brand> (which defines what you can DO, not what it IS).

    What matters is that as this is what you expose to the caller this is only what you guarantee to them. If client code is casting back to List<Brand> it "breaks" the contract by assuming more than your actual engagement.

    Similarly you can access private members by writing additional code. The goal is not to absolutely prevent breaking the rules but that rules are enforced by default unless your code is EXPLICITELY breaking those rules.

    Thursday, February 14, 2019 7:34 PM
  • User475983607 posted

    Is it correct to use a List<T> and return it?

    Sure because List<T> inherits from IEnumerable<T>.  Essentially List<T> can do everything IEnumerable<T> can do.

    Isn´t is the same if I set the return type as List<Brand> instead of IEnumerable<Brand>?

    You are currently returning List<T>.  If you are asking about the method return type you can change the return type to List<T> but that can be restrictive in your repository design.  

    What is the advantage of setting the return type as IEnumerable<Brand> in the method?

    You can return any type that implements IEnumerable<T>.  

    • Marked as answer by Anonymous Thursday, October 7, 2021 12:00 AM
    Thursday, February 14, 2019 8:04 PM