tags:

views:

128

answers:

7

Im trying to add data in a dataset into a List. This is my function in C#

  public List<ProductsDAL> GetAllProducts(string sqlQuery)
  {
     DataSet dsinsert = GetDataSet(sqlQuery, "tblProducts");
     List<ProductsDAL> bPList = new List<ProductsDAL>();
ProductsDALforeach (DataRow item in dsinsert.Tables[0].Rows)
     {
       this.Product_ID = item["Product_ID"].ToString();
       this.ProductDescr= item["ProductDescr"].ToString();
       bPList.Add(this);
     }
     return bPList;
  }

The result in the dataset is like

column1 - colum2
A         1 
B         2
C         3
D         4

But I think the result in the list is:

column1 - colum2
D         1 
D         1
D         1
D         1

When I insert this set of data into another database I only get this:

column1 - colum2
D         1 

What am I doing wrong in my function ?

+2  A: 

You are adding the current object, this, to the list and then, in subsequent iterations, you are modifying the same object (via the this reference) before adding it, again, to the list. This means you end up with a list containing the same object (the this object) over and over again. Its values will reflect only the latest values applied to it.

What you want to do instead is add new instances of ProductsDAL to the list each time, i.e. instead of modiying this, instead you should create a new ProductsDAL, set its state and add that to the list.

Here is a the first change you should make:


public List GetAllProducts(string sqlQuery)
  {
     DataSet dsinsert = GetDataSet(sqlQuery, "tblProducts");
     List bPList = new List();
     foreach (DataRow item in dsinsert.Tables[0].Rows)
     {
       ProductsDAL product = new ProductsDAL();
       product.Product_ID = item["Product_ID"].ToString();
       product.ProductDescr = item["ProductDescr"].ToString();
       bPList.Add(product);
     }
     return bPList;
  }

Also:

  1. Because this method now has no effect on the current ProductsDAL instance (it's actually creating new ones) then it not longer makes sense for it to be a instance method. You could consider making it a static method (and making GetDataSet() static too) or moving this logic out into some other class that has the responsibility of fetching products from the database.
  2. You may want to change the constructor on ProductsDAL so that the mandatory fields have to be set so that it is not possible to create ProductsDAL objects that do not have their ID and description set.
Paul Ruane
+2  A: 

You need to create a new ProductsDAL inside the foreach statement. You're just updating the same one.

Aaron Daniels
+2  A: 

I think your problem is your usage of this. My guess is you are doing this in your Product Class

change it to this

public List<ProductsDAL> GetAllProducts(string sqlQuery)
  {
     DataSet dsinsert = GetDataSet(sqlQuery, "tblProducts");
     List<ProductsDAL> bPList = new List<ProductsDAL>();
     ProductsDAL p = null;
     ProductsDALforeach (DataRow item in dsinsert.Tables[0].Rows)
     {
       p =new ProductsDAL();
       p.Product_ID = item["Product_ID"].ToString();
       p.ProductDescr= item["ProductDescr"].ToString();
       bPList.Add(p);
     }
     return bPList;
  }
freddoo
A: 
1. public List<ProductsDAL> GetAllProducts(string sqlQuery)
2. {
3.     DataSet dsinsert = GetDataSet(sqlQuery, "tblProducts");
4.     List<ProductsDAL> bPList = new List<ProductsDAL>();
5.     ProductsDALforeach (DataRow item in dsinsert.Tables[0].Rows)
6.     {
7.       this.Product_ID = item["Product_ID"].ToString();
8.       this.ProductDescr= item["ProductDescr"].ToString();
9.       bPList.Add(this);
10.     }
11.    return bPList;
12. }

I think line 9 might be causing problems. Instead (to replace lines 7-9), I would probably do something along the lines of:

ProductDAL productDal = new ProductsDAL(){
                          Product_ID = item["Product_ID"].ToString(),
                          ProductDescr =  item["ProductDescr"].ToString()};
bPList.Add(productDal);
Jamie Chapman
A: 

You are reusing the same object over and over, you need to re-create that object. I like seperating that code in another function. Here is how I would do it:

First you should getproducts like so:

public IList<Product> GetProducts(string sortexpression)
{
  StringBuilder sql = new StringBuilder();
            sql.Append(" SELECT ProductName, ProductID");
            sql.Append("   FROM Products ");
            if (!string.IsNullOrEmpty(sortExpression))
                sql.Append(" ORDER BY " + sortExpression);

            DataTable dt = Db.GetDataTable(sql.ToString());

            return MakeProducts(dt);
}

So you may ask how does Db.GetDataTable look, well like this:

 public static DataTable GetDataTable(string sql)
        {
            using (DbConnection connection = factory.CreateConnection())
            {
                connection.ConnectionString = connectionString;

                using (DbCommand command = factory.CreateCommand())
                {
                    command.Connection = connection;
                    command.CommandType = CommandType.Text;
                    command.CommandText = sql;

                    using (DbDataAdapter adapter = factory.CreateDataAdapter())
                    {
                        adapter.SelectCommand = command;

                        DataTable dt = new DataTable();
                        adapter.Fill(dt);

                        return dt;
                    }
                }
            }

For simplicity, I am not using sprocs, but you should...anyhow you may ask what does MakeProducts(dt) look like, and that is simply a function to loop and add to a list:

   private IList<Product> MakeProducts(DataTable dt)
    {
        IList<Product> list = new List<Product>();
        foreach (DataRow row in dt.Rows)
            list.Add(MakeProduct(row));

        return list;
    }

So here you simply loop each row and make a product object which looks like this:

 private Product MakeProduct(DataRow row)
        {
            int productId = int.Parse(row["ProductId"].ToString());
            string name = row["ProductName"].ToString();

            return new Product(productId, name);
        }
JonH
A: 

Would the following work?

public List<ProductsDAL> GetAllProducts(string sqlQuery) 
{ 
    return GetDataSet(sqlQuery, "tblProducts").
        Tables[0].Rows.Cast<System.Data.DataRow>().
        Select(p => new ProductsDAL()
        {
            Product_ID = p["Product_ID"].ToString(),
            ProductDescr = p["ProductDescr"].ToString()
        }).ToList();    
} 
Kelsey
A: 

Another option is to use LINQ to DataSets...

public List<ProductsDAL> GetAllProducts(string sqlQuery) {
    DataSet dsinsert = GetDataSet(sqlQuery, "tblProducts");

    return (from row in dsinsert.Tables[0].AsEnumerable()
            select new ProductsDAL {
                Product_ID = row.Field<string>("Product_ID"),
                ProductDescr = row.Field<string>("ProductDescr"),
            }).ToList();
}
Tom Brothers