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

DAL Returns a collection of business objects in MVP

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
businessobjectsdalcollectionmvpreturns

Problem

In Model View Presenter (MVP) pattern, it is said that our DAL always should returns business models.

-
If I want to get a list of business objects from my DAL, is it acceptable to do it as follows or better to get a DataTable from DAL and then in the applications BLL, converts it to a List?

public List  GetNewEmployees()
{
    string selectStatement = "SELECT Employee.Emp_ID, Employee.Initials + ' ' + Employee.Surname AS Name,...";

    using (SqlConnection sqlConnection = new SqlConnection(db.GetConnectionString))
    {
        using (SqlCommand sqlCommand = new SqlCommand(selectStatement, sqlConnection))
        {
            sqlConnection.Open();
            using (SqlDataReader dataReader = sqlCommand.ExecuteReader())
            {
                List list = new List();
                while (dataReader.Read())
                {
                    list.Add (
                    new EpfEtfMaster { 
                        EmployeeID = (int) dataReader ["emp_id"],
                        EmployeeName = (string) dataReader ["Name"],
                        AppointmentDate = (DateTime) dataReader["appointment_date"],                                   
                    });                            
                }
                return list;
            }
        }
    }
}

Solution

You don't want to pull the entire Employee table unless necessary.

public IEnumerable  GetNewEmployees()
{
    string selectStatement = "SELECT Employee.Emp_ID, Employee.Initials + ' ' + Employee.Surname AS Name,...";

    using (SqlConnection sqlConnection = new SqlConnection(db.GetConnectionString))
    {
        using (SqlCommand sqlCommand = new SqlCommand(selectStatement, sqlConnection))
        {
            sqlConnection.Open();
            using (SqlDataReader dataReader = sqlCommand.ExecuteReader())
            {
                while (dataReader.Read())
                {
                    yield return new EpfEtfMaster(){ 
                        EmployeeID = (int) dataReader ["emp_id"],
                        EmployeeName = (string) dataReader ["Name"],
                        AppointmentDate = (DateTime) dataReader["appointment_date"],                                   
                    };
                }
            }
        }
    }
}


By switching to IEnumerable and yield return, when you call GetNewEmployees().First(), only the first row is read.

  • The names like sqlCommand, sqlConnection and dataReader can be shortened to command, connection and reader without affecting the reader.



  • Try to not repeat yourself with SomeThing someThing = new SomeThing(...);. The first class name can be replaced by using var.



Result :

using (var connection = new SqlConnection(db.GetConnectionString))


new SqlConnection(db.GetConnectionString)


Use verb for method and noun for field/properties. Here GetX suggest it is a method but is used like a property, you can rename it to ConnectionString.

Code Snippets

public IEnumerable<Employee>  GetNewEmployees()
{
    string selectStatement = "SELECT Employee.Emp_ID, Employee.Initials + ' ' + Employee.Surname AS Name,...";

    using (SqlConnection sqlConnection = new SqlConnection(db.GetConnectionString))
    {
        using (SqlCommand sqlCommand = new SqlCommand(selectStatement, sqlConnection))
        {
            sqlConnection.Open();
            using (SqlDataReader dataReader = sqlCommand.ExecuteReader())
            {
                while (dataReader.Read())
                {
                    yield return new EpfEtfMaster(){ 
                        EmployeeID = (int) dataReader ["emp_id"],
                        EmployeeName = (string) dataReader ["Name"],
                        AppointmentDate = (DateTime) dataReader["appointment_date"],                                   
                    };
                }
            }
        }
    }
}
using (var connection = new SqlConnection(db.GetConnectionString))
new SqlConnection(db.GetConnectionString)

Context

StackExchange Code Review Q#52251, answer score: 6

Revisions (0)

No revisions yet.