patterncsharpMinor
Overly interfaced or not overly interfaced?
Viewed 0 times
notinterfacedoverly
Problem
I have to take over an old project that was coded in VS 2010 C# 4.0 in 2012. Interface programming seems to be all over the place and over-applied to me, but I am unsure if it is that way.
I need to know what to keep and what to refactor.
Looking at the code, do you think it was applied correctly? Interface is used as a contract, ain't it?
```
//CustomerBLL.cs
namespace Inventory.BLL
{
class CustomerBLL : ICustomerBLL
{
#region Services
ICustomerDAL CustomerDAL
{
get { return DALProvider.CustomerDAL; }
}
#endregion
#region ICustomerBLL Members
public Customer GetCustomerDetail(string customerCode)
{
return CustomerDAL.GetCustomerDetail(customerCode);
}
public bool Save(Customer customer)
{
return CustomerDAL.Save(customer);
}
public DataTable GetCustomers_List(Customer customer)
{
return CustomerDAL.GetCustomers_List(customer);
}
#endregion
}
}
//End of Class
//Interface ICustomerBLL.cs
namespace Inventory.BLL
{
public interface ICustomerBLL
{
Customer GetCustomerDetail(string customerCode);
bool Save(Customer customer);
DataTable GetCustomers_List(Customer customer);
}
}
//End of Interface
//BLLProvider.cs
namespace Inventory.BLL
{
public class BLLProvider
{
private static ICustomerBLL customerBLL = new CustomerBLL();
public static ICustomerBLL CustomerBLL
{
get { return customerBLL; }
}
}
}
//End of Class
//Customer.cs
namespace Inventory.BO
{
[Serializable]
public class Customer : CloneBa
I need to know what to keep and what to refactor.
Looking at the code, do you think it was applied correctly? Interface is used as a contract, ain't it?
Solution Explorer
===========================
Inventory.BLL
|_ CustomerBLL.cs
|_ ICustomerBLL.cs
|_ BLLProvider.cs
Inventory.BO
|_ Customer.cs
Inventory.DAL
|_ CustomerDAL.cs
|_ ICustomerDAL.cs
|_ DALProvider.cs
Inventory.UI
|_ frmCustomer.cs
===========================```
//CustomerBLL.cs
namespace Inventory.BLL
{
class CustomerBLL : ICustomerBLL
{
#region Services
ICustomerDAL CustomerDAL
{
get { return DALProvider.CustomerDAL; }
}
#endregion
#region ICustomerBLL Members
public Customer GetCustomerDetail(string customerCode)
{
return CustomerDAL.GetCustomerDetail(customerCode);
}
public bool Save(Customer customer)
{
return CustomerDAL.Save(customer);
}
public DataTable GetCustomers_List(Customer customer)
{
return CustomerDAL.GetCustomers_List(customer);
}
#endregion
}
}
//End of Class
//Interface ICustomerBLL.cs
namespace Inventory.BLL
{
public interface ICustomerBLL
{
Customer GetCustomerDetail(string customerCode);
bool Save(Customer customer);
DataTable GetCustomers_List(Customer customer);
}
}
//End of Interface
//BLLProvider.cs
namespace Inventory.BLL
{
public class BLLProvider
{
private static ICustomerBLL customerBLL = new CustomerBLL();
public static ICustomerBLL CustomerBLL
{
get { return customerBLL; }
}
}
}
//End of Class
//Customer.cs
namespace Inventory.BO
{
[Serializable]
public class Customer : CloneBa
Solution
CustomerDAL
You can remove the
Customer
By using auto properties this can be reduced to
You can remove the
BindCustomer() method as it isn't called at all. But if you want to keep it, because you didn't show the whole code, you can, by extracting the childrow processing to a separate method, reduce it to private Customer BindCustomer(DataSet ds, DataRelation dr)
{
Customer customer = new Customer();
int rowCount =ds.Tables[0].Rows.Count;
if (rowCount == 0) {return customer;}
DataRow row = ds.Tables[0].Rows[rowCount -1];
customer.CustomerCode = Convert.ToString(row[0]);
customer.Address1 = Convert.ToString(row[4]);
customer.MailingAddresses = GetMailingAddresses(row.GetChildRows(dr));
return customer;
}
private List GetMailingAddresses(DataRow[] childRows)
{
Listaddresses = new List();
foreach (DataRow addressRow in childRows)
{
MailingAddresses mailingAddresses = new MailingAddresses();
mailingAddresses.CustomerCode = Convert.ToString(addressRow[1]);
mailingAddresses.Address1 = Convert.ToString(addressRow[4]);
addresses.Add(mailingAddresses);
}
return addresses;
}Customer
By using auto properties this can be reduced to
public class Customer : CloneBase
{
public string CustomerCode { get; set;}
public string Address1 { get; set;}
}Code Snippets
private Customer BindCustomer(DataSet ds, DataRelation dr)
{
Customer customer = new Customer();
int rowCount =ds.Tables[0].Rows.Count;
if (rowCount == 0) {return customer;}
DataRow row = ds.Tables[0].Rows[rowCount -1];
customer.CustomerCode = Convert.ToString(row[0]);
customer.Address1 = Convert.ToString(row[4]);
customer.MailingAddresses = GetMailingAddresses(row.GetChildRows(dr));
return customer;
}
private List<MailingAddresses> GetMailingAddresses(DataRow[] childRows)
{
List<MailingAddresses>addresses = new List<MailingAddresses>();
foreach (DataRow addressRow in childRows)
{
MailingAddresses mailingAddresses = new MailingAddresses();
mailingAddresses.CustomerCode = Convert.ToString(addressRow[1]);
mailingAddresses.Address1 = Convert.ToString(addressRow[4]);
addresses.Add(mailingAddresses);
}
return addresses;
}public class Customer : CloneBase<Customer>
{
public string CustomerCode { get; set;}
public string Address1 { get; set;}
}Context
StackExchange Code Review Q#68242, answer score: 3
Revisions (0)
No revisions yet.