-1

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)
        }
    }

    // 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);
        }
    }
skaffman
  • 398,947
  • 96
  • 818
  • 769
balun
  • 7
  • 1
  • 1
    Do you have any specific questions about your architecture? What are your reasons for using multiple tiers? Do you understand [the difference between a tier and a layer](http://stackoverflow.com/q/120438/310112)? – Joel C Aug 16 '11 at 19:17

2 Answers2

2

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);
   }
}
sll
  • 61,540
  • 22
  • 104
  • 156
2
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.

Chris Marisic
  • 32,487
  • 24
  • 164
  • 258