Multi tier program architecture, feedback and/or suggestions [closed]

This question is unlikely to help any future visitors; it is only relevant to a small geographic area, a specific moment in time, or an extraordinarily narrow situation that is not generally applicable to the worldwide audience of the internet. For help making this question more broadly applicable, visit the help center. Closed 11 years ago.

I'm planning the structure of a program and want to use several tiers. Do you think my approach is good or do you have some other suggestions?

    // The Form is the View + Controller (Windows Forms standard behaviour, don't want to change it)
    class FormCustomer
    {
        CustomerModel _customerModel;
        void LoadCustomer()
        {
            Customer c = _customerModel.ReadCustomer(tbCustomer.Text);
            ShowCustomer(c);
        }
    }

    // The Model-Layer is for business Logic
    class CustomerModel
    {
        StorageLayer _StorageLayer;        
        public Customer ReadCustomer(int id)
        {
            if (id < 0) throw new Exception("Invalid id");
            Customer c = _StorageLayer.ReadCustomer(id);
            if (c == null) throw new Exception("Customer not found");
            return c;
        }
    }

    // The StorageLayer ist a facade to all storage Methods
    // See http://en.wikipedia.org/wiki/Facade_pattern for more details
    class StorageLayer
    {
        SqlMethods _sqlMethods;
        public Customer ReadCustomer(int id)
        {
            return _sqlMethods.ReadCustomer(id)
        }
    }
开发者_StackOverflow社区
    // The SqlMethods is one class (or maybe several classes) which contain
    // all the sql operations.
    class SqlMethods
    {
        public Customer ReadCustomer(int id)
        {
            string sql = "Select c.*, a.* From customers c left join addresses a where c.id = " + id; // Not optimized, just an example
            IDataReader dr = ExecuteStatement(sql);
            return FetchCustomer(dr);
        }
    }

---

string sql = "Select c.*, a.* From customers c left
                    join addresses a where c.id = " + id; 
// Not optimized, just an example IDataReader dr
= ExecuteStatement(sql);

NEVER EVER DO THIS FOR ANY REASON. This is completely unacceptable. It is not acceptable for testing, it is not acceptable for a prototype. Doing this for any reason as a developer is chop your hand off punishment worthy.

This is sql injection at it's finest.

Just because this is an int now doesn't mean you won't change it to a string, or realize you need some other parameter then join in the string.

You MUST use parameters.

---

1) First problem - tied coupling.

• FormCustomer to CustomerModel
• CustomerModel to StorageLayer, Customer
• StorageLayer to Customer, SqlMethods
• TODO: Introduce interfaces and inject dependencies on construction stage

 // Now you don't need StorageLayer, basically it would be IDataService
 // rModel class should provide a some kind of business logic otherwise it just 
 // wrapping with zero value a IDataService and will have 
 // a mess introducing model class per entity
 public sealed class CustomerModel 
 {
    private readonly IDataService 

    // now you can inject any an other data service without model changes
    // XmlDataService, WebDataService, etc
    public CustomerModel (IDataService dataService)
    {
        this.dataService = dataService;
    }

    public ICustomer GetCustomer(int id)
    {
        if (id < 0) 
        {
           throw new ArgumentException("id", 
                                  "Id should be equal or greater than zero");
        }

        ICustomer c = this.dataService.SelectEntity<Customer>(id);
        // ...
    }
 }

2) Second - try to use generics, so each time you need a new entity along with Customer, like Account, etc you can reuse at least a major part of infrastructure.

TODO: Consider design like to decouple queries from entities (perhaps sometime it woudl not be an SQL queries?)

public interface IDbEntity 
{
}

public interface IDbContract
{
      string SelectByIdQuery { get; }
}

public sealed class DataBaseContractFactory
{
    public IDbContract CreateContract<TEntity>()
      where TEntity: IDbEntity
    {
       if (typeof(TEntity) == typeof(ICustomer))
       {
           return this.customerDbContract;
       }
    }
}

public sealed class SqlDataService: IDataService
{
   public SqlDataService(DataBaseContractFactory dbContractFactory)
   {
        // assign to private field
   }

   public T SelectEntity<TEntity>(int entityId)
      where TEntity: IDbEntity
   {
       IDbContract contract = this.dbContractFactory.CreateContract<TEntity>();

       // Consider using more safe way of query building:
       // adapter.SelectCommand.Parameters.Add(
       // "@Id", SqlDbType.Int).Value = id;
       string sqlQuery = String.Format(contract.SelectByIdQuery, id);

       IDataReader dataReader = ExecuteStatement(sqlQuery);
       return this.BuildEntytyFromDataReader<TEntity>(dataReader);
   }
}