3
\$\begingroup\$
 public class EmployeeDBHandler
    {
        public List<Employee> GetEmployees()
        {
            List<Employee> empList = new List<Employee>();
            string sqlConnstr = Utils.GetDBConnection();
            SqlConnection sqlConn = new SqlConnection(sqlConnstr);
            SqlCommand sqlCmd = new SqlCommand("GetAllEmployee", sqlConn);
            sqlCmd.CommandType = CommandType.StoredProcedure;
            sqlConn.Open();
            using (SqlDataReader reader = sqlCmd.ExecuteReader())
            { 
                while (reader.Read())
                {
                    var emp = new Employee()
                    {
                        EId = (int)(reader["EmpID"]),
                        FirstName = Convert.ToString(reader["FirstName"]),
                        LastName = Convert.ToString(reader["LastName"])                        
                    };
                    empList.Add(emp);
                }
            }
            return empList;
        }
        public List<Employee> FetchEmployee(int empid)
        {
            List<Employee> empList = new List<Employee>();
            string sqlConnstr = Utils.GetDBConnection();
            SqlConnection sqlConn = new SqlConnection(sqlConnstr);
            SqlCommand sqlCmd = new SqlCommand("usp_FetchEmployeeDetails", sqlConn);
            sqlCmd.CommandType = CommandType.StoredProcedure;
            sqlCmd.Parameters.AddWithValue("@EmpID", empid);
            sqlConn.Open();
            using (SqlDataReader reader = sqlCmd.ExecuteReader())
            {
                while (reader.Read())
                {
                    var emp = new Employee()
                    {
                        EId = (int)(reader["EmpID"]),
                        FirstName = (reader["FirstName"].ToString()),
                        LastName = (reader["LastName"].ToString())
                    };
                    empList.Add(emp);
                }
            }
            return empList;
        }
        public void AddEmployeeInfo(string firstname, string lastname)
        {
            string sqlConnstr = Utils.GetDBConnection();
            SqlConnection sqlConn = new SqlConnection(sqlConnstr);
            SqlCommand sqlCmd = new SqlCommand("AddEmployee", sqlConn);
            sqlCmd.CommandType = CommandType.StoredProcedure;
            sqlCmd.Parameters.AddWithValue("@FirstName", firstname);
            sqlCmd.Parameters.AddWithValue("@LastName", lastname);
            sqlConn.Open();
            sqlCmd.ExecuteNonQuery();
            sqlConn.Close();
        }
        public void UpdateEmployee(int empid,string firstname, string lastname)
        {
            string sqlConnstr = Utils.GetDBConnection();
            SqlConnection sqlConn = new SqlConnection(sqlConnstr);
            SqlCommand sqlCmd = new SqlCommand("usp_UpdateEmployeeDetails", sqlConn);
            sqlCmd.CommandType = CommandType.StoredProcedure;
            sqlCmd.Parameters.AddWithValue("@EmpID", empid);
            sqlCmd.Parameters.AddWithValue("@FirstName", firstname);
            sqlCmd.Parameters.AddWithValue("@LastName", lastname);
            sqlConn.Open();
            sqlCmd.ExecuteNonQuery();
            sqlConn.Close();
        }
        public void DeleteEmployee(int empid)
        {
            string sqlConnstr = Utils.GetDBConnection();
            SqlConnection sqlConn = new SqlConnection(sqlConnstr);
            SqlCommand sqlCmd = new SqlCommand("usp_DeleteEmployeeDetails", sqlConn);
            sqlCmd.CommandType = CommandType.StoredProcedure;
            sqlCmd.Parameters.AddWithValue("@EmpID",empid);
            sqlConn.Open();
            sqlCmd.ExecuteNonQuery();
            sqlConn.Close();
        }

    }

I'm try to build a 3-tier arch. but I feel these code can be improvised so please review my above Data Access Layer code and give suggestions and corrections, if any, to improvise my code.

\$\endgroup\$
2
  • 5
    \$\begingroup\$ This is riddled with outdated and downright bad practices. Throw it all away and use Dapper instead. \$\endgroup\$ – BCdotWEB Feb 24 '20 at 11:38
  • \$\begingroup\$ @BCdotWEB Thanks for the suggestion.. minimal code and neat..It's Dapper. \$\endgroup\$ – Krish Feb 26 '20 at 11:41
-1
\$\begingroup\$

something along these lines

public static class DTExtensions
{
    public static List<T> ToList<T>(this System.Data.DataTable dt) where T : new()
    {
        var obj = dt.Rows.OfType<System.Data.DataRow>().Select(dr => dr.ToObject<T>()).ToList();
        return obj;
    }
    public static T ToObject<T>(this System.Data.DataRow dataRow) where T : new()
    {
        T item = new T();
        var itemType = item.GetType();
        foreach (System.Data.DataColumn column in dataRow.Table.Columns)
        {
            System.Reflection.PropertyInfo property = itemType.GetProperty(column.ColumnName);

            if (property != null && dataRow[column] != DBNull.Value)
            {
                var result = Convert.ChangeType(dataRow[column], property.PropertyType);
                property.SetValue(item, result, null);
            }
        }

        return item;
    }
}


public class DataHandler
{
    private readonly string _ConnectionInfo = String.Empty;
    public DataHandler(string connectioninfo)
    {
        _ConnectionInfo = connectioninfo;
    }

    public async Task<System.Data.DataTable> DataAsync( string commandSQL, IEnumerable<System.Data.SqlClient.SqlParameter> listParameter=null)
    {
        System.Data.DataTable dt = new System.Data.DataTable("data");

        using (System.Data.SqlClient.SqlConnection connection = new System.Data.SqlClient.SqlConnection(_ConnectionInfo))
        {
            try
            {
                await connection.OpenAsync().ConfigureAwait(false);
            }
            catch (InvalidOperationException ex)
            {
                return null;
            }
            catch (System.Data.SqlClient.SqlException ex)
            {
                return null;
            }
            using (System.Data.SqlClient.SqlCommand command = new System.Data.SqlClient.SqlCommand(commandSQL, connection))
            {
                command.CommandType = System.Data.CommandType.StoredProcedure;
                if (listParameter != null && listParameter.Any())
                {
                    command.Parameters.AddRange(listParameter.Where(p => p != null).ToArray());
                }
                using (var r = await command.ExecuteReaderAsync(System.Data.CommandBehavior.SequentialAccess).ConfigureAwait(false))
                {
                    dt.Load(r);
                }
                command.Parameters.Clear();
            }
            connection.Close();
        }

        return dt;
    }
}

public sealed class Employee
{
    public string EmpID { get; set; }
    public string FirstName { get; set; }
    public string LastName { get; set; }
}

public class EmployeeDBHandler
{
    private readonly DataHandler _DataHandler;
    public EmployeeDBHandler(DataHandler datahandler)
    {
        _DataHandler = datahandler ?? throw new System.ArgumentNullException("datahandler");
    }

    public async Task<List<Employee>> GetEmployees()
    {
        System.Data.DataTable dt = await _DataHandler.DataAsync("GetAllEmployee").ConfigureAwait(false);
        if (dt != null)
        {
            //OR look into using yeild
            //yield return employee
            //OR
            var list = dt.ToList<Employee>();
            return list;
        }
        return null;
    }

    public async Task<List<Employee>> FetchEmployee(int empid)
    {
        List<Employee> empList = new List<Employee>();
        var parameters = new List<System.Data.SqlClient.SqlParameter> { new SqlParameter("@EmpID", empid) };
        System.Data.DataTable dt = await _DataHandler.DataAsync("usp_FetchEmployeeDetails", parameters).ConfigureAwait(false);
        if (dt != null && dt.Rows.Count>0)
        {
            var list = dt.ToList<Employee>();
            return list;
        }
        return null;
    }
}

//  main program:
public class MainProgram
{
    public void DoSomeWork()
    {
        string sqlConnstr = @"Data Source=<...>; Initial Catalog=<...>; User ID=username; Password=password"; //Utils.GetDBConnection();
        var dbHandler = new DataHandler(sqlConnstr);
        var employeeHandler = new EmployeeDBHandler(dbHandler);

        var employees = employeeHandler.GetEmployees();
        if (employees != null)
        {
            //.....
        }
    }
}
\$\endgroup\$
5
  • 1
    \$\begingroup\$ Thanks for your effort. Could you edit your code by adding some comments which will give me more understanding on the same. \$\endgroup\$ – Krish Feb 26 '20 at 11:44
  • 1
    \$\begingroup\$ -1 This provides NO review at all, just a bunch of code without an explanation on what it does improve and no rationale for any change. \$\endgroup\$ – Alejandro Mar 27 '20 at 13:46
  • \$\begingroup\$ this provides "give suggestions and corrections". \$\endgroup\$ – yob Mar 27 '20 at 13:57
  • 1
    \$\begingroup\$ @yob No, it doesn't. It's just a bunch of code, with no suggestion nor correction of anything. You should explain the rationale for the suggested code and how it improves the original code. A code review is all about explaining possible improvements, not just "use this instead". \$\endgroup\$ – Alejandro Mar 27 '20 at 15:17
  • 1
    \$\begingroup\$ This is not a code review. \$\endgroup\$ – Adrian Iftode Nov 21 '20 at 22:32

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.