Display items from DB using MVC and Domain Driven Design

Question

I am starting a project to put into practice the lessons learned in "Clean Code" by Robert "Uncle Bob" Martin.

The project is a website that displays the names of items in a DB table. It uses ASP.NET MVC Framework, and I have tried my best to separate domains in the Model. I have used Test Driven Design, but there is a lot of code already, so I have left it out here.

It is extremely basic, but it uses a lot of concepts, and I would like to make sure I have understood everything right before adding complexity.

I am interested in learning how I could have better followed Agile concepts, even if overkill for this little application (or course, any other advice is good to take). In particular, I feel like to controller is doing a lot of logic for a single function (and it is difficult to unit test).

DataAccessLayer.dll

Customer.cs

using System;
using System.Collections.Generic;
using System.Linq;
using System.Text;
using System.Threading.Tasks;

namespace DataAccessLayer.DataTransferObject
{
    public class Customer
    {
        public string Name { get; set; }
        public Customer(string inputName)
        {
            Name = inputName;
        }
    }
}

ICustomerRepository.cs

using System;
using System.Collections.Generic;
using System.Data;
using System.Linq;
using System.Text;
using System.Threading.Tasks;

namespace DataAccessLayer.Repositories.Interfaces
{
    public interface ICustomerRepository
    {
        ICollection<DataTransferObject.Customer> GetAllCustomerss();
    }
}

SQLPlusCustomerRepository.cs

using System;
using System.Collections.Generic;
using System.Linq;
using System.Text;
using System.Threading.Tasks;
using DataAccessLayer.DataTransferObject;
using System.Data;

namespace DataAccessLayer.Repositories
{
    public class SQLPlusCustomerRepository : Interfaces.ICustomerRepository
    {
        private IDbConnection connection;
        public SQLPlusCustomerRepository(IDbConnection inputConnection)
        {
            connection = inputConnection;
        }
        public ICollection<Customer> GetAllCustomers()
        {
            ICollection<Customer> resultList = new List<Customer>();

            const string getAllCustomerQuery = "SELECT NAME FROM CUSTOMERS";

            using (connection)
            {
                connection.Open();
                IDbCommand dbCommand = connection.CreateCommand();
                dbCommand.CommandType = CommandType.Text;
                dbCommand.CommandText = getAllCustomerQuery;

                IDataReader dbDataReader = dbCommand.ExecuteReader();
                while (dbDataReader.Read())
                {
                    resultList.Add(new Customer(dbDataReader.GetValue(0).ToString()));
                }
                return resultList;
            }
        }
    }
}

CustomerDisplay (WebSite)

Customer.cs

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

namespace CustomerDisplay.Models
{
    public class Customer
    {
        public string Name { get; set; }
        public Customer(DataAccessLayer.DataTransferObject.Customer dtoCustomer)
        {
            Name = dtoCustomer.Name;
        }
    }
}

HomeController.cs

using System;
using System.Collections.Generic;
using System.Linq;
using System.Web;
using System.Web.Mvc;
using XXX.SQLplus;

namespace CustomerDisplay.Controllers
{
    public class HomeController : Controller
    {
        public ActionResult Index()
        {
            SQLplusConnection connection = new SQLplusConnection(
                Properties.Settings.Default.connectionString
                );
            DataAccessLayer.Repositories.SQLPlusCustomerRepository repository =
                new DataAccessLayer.Repositories.SQLPlusCustomerRepository(connection);

            ICollection<Models.Customer> viewItemsList = new List<Models.Customer>();

            foreach (DataAccessLayer.DataTransferObject.Customer dtoCustomer in repository.GetAllCustomers()){
                viewItemsList.Add(new Models.Customers(dtoCustomers));
            }
            return View(viewItemsList);
        }
    }
}

Index.cshtml

@model  ICollection<CustomerDisplay.Models.Customer>

@foreach(CustomerDisplay.Models.Customer Customer in Model)
{
    <span>@Customer.Name</span>
}

Answer 1

Give proper names to parameters; don't pointlessly prefix them, e.g. "inputName" or "dtoCustomer".

---

Four capital letters in a row -- as in "SQLPlus" -- goes against Microsoft's coding guidelines. It is expressed at length in the old version:

Do capitalize only the first character of acronyms with three or more characters, except the first word of a camel-cased identifier.

---

Your GetAllCustomers() method is 20 lines for something that can be done much easier and neater when using an ORM like Entity Framework or Dapper. IMHO you should avoid writing ADO.NET code unless you have a really good reason to do it.

I also have the impression that you don't always properly clean up data readers etc.

---

Is there a reason why your code is littered with class name prefixes like "Interfaces." or "DataAccessLayer.DataTransferObject."? Those make your code harder to read, IMHO.

---

I don't see why you create a SQLplusConnection in your controller, only to pass to to the SQLPlusCustomerRepository? Why not let the repository deal with all of that instead of mixing it all over the place?

---

The foreach converting DTOs to Models could be done far more elegantly in LINQ.

---

There are 100+ lines of code here, and all you've done is retrieved one field from one table and displayed that. I'm all in favor of separation, but to me this feels like massive overkill and it is likely to become very unwieldy very soon. It looks like code that would have been barely acceptable ten+ years ago, these days it feels outdated and bloated.