HiveBrain v1.2.0
Get Started
← Back to all entries
patterncsharpMinor

Overly interfaced or not overly interfaced?

Submitted by: @import:stackexchange-codereview··
0
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?

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 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.