CODEWITHSUNDEEP
c#.netasp.netsqlgridview
Naresh
Naresh

Reputation: 657

Problem in populating the GridView in ASP.NET(C#)

I'm trying to populate a Gridview with results from loop. But I'm getting only last result in the loop.
I think GridView is being overwritten on every time the for loop is being executed.

Can you people help me to remove this problem please.

for (int j = 0; j < i; j++)
{
    Label1.Text += fipath[j];
    Label1.Text += "-------------";
    SqlConnection conn = new SqlConnection("Server=ILLUMINATI;" + "Database=DB;Integrated Security= true");
    SqlCommand comm = new SqlCommand("Select * from FileUpload where UploadedBy='" + NAME + "' AND FilePath='" + fipath[j] + "'", conn);

    try
    {
        conn.Open();
        SqlDataReader rdr = comm.ExecuteReader();
        if (Role.Equals("admin"))
        {
            GridView1.DataSource = rdr;
            GridView1.DataBind();
        }
        rdr.Close();
    }
    catch
    {
        conn.Close();
    }
}

Upvotes: 2

Views: 623

Answers (3)

Muhammad Akhtar
Muhammad Akhtar

Reputation: 52241

Use the in clause in SQL Query and pass the list of ID's in FilePath

SqlCommand comm = new SqlCommand("Select * from FileUpload where UploadedBy='" + NAME 
 + "' AND FilePath in (" + listOfIDs + ")", conn);

Check out these URLs that are related to the use of in clause.

Techniques for In-Clause and SQL Server

Parameterizing a SQL IN clause?

Upvotes: 2

Alex Aza
Alex Aza

Reputation: 78457

There is more than one problem with this code:

  • seems like if Role== "admin" you don't need to query db at all
  • DataSource of the grid is overridden on every loop iteration, this is why you see only the last value.
  • use parameters for SqlCommand to prevent SQL injection.
  • don't run string concatenation in the loop. Use StringBuilder instead
  • use using for your connection. The code is cleaner this way.

The fix could look like this:

if (Role != "admin")
    return;

var dataTable = new DataTable();
var stringBuilder = new StringBuilder();
using (var connection = new SqlConnection("Server=ILLUMINATI;" + "Database=DB;Integrated Security= true"))
using (var command = connection.CreateCommand())
{
    connection.Open();
    command.CommandText = "Select * from FileUpload where UploadedBy = @UploadedBy AND FilePath = @FilePath";
    command.Parameters.AddWithValue("UploadedBy", NAME);
    var filPathParameter = command.Parameters.Add("FilePath", SqlDbType.VarChar);
    for (int j = 0; j < i; j++)
    {
        stringBuilder.Append(fipath[j]);
        stringBuilder.Append("-------------");
        filPathParameter.Value = fipath[j];
        dataTable.Load(command.ExecuteReader(), LoadOption.PreserveChanges);
    }
}
Label1.Text += stringBuilder.ToString();
GridView1.DataSource = dataTable;
GridView1.DataBind();

Also, I don't know how many elements your normal loop is. If it is one or two and you have appropriate indexes in FileUpload table then it is ok to leave as is. However, if you need to do the for many times you should consider switching to a single query instead

For example:

var filePathes = string.Join(",", fipath.Select(arg => "'" + arg + "'"));
var command = "Select * from FileUpload where UploadedBy = @UploadedBy AND FilePath in (" + filePathes + ")";

This query is SQL injection prone. And has a 2100 elements limit in MS SQL.

There is more than one way to approach this. Depends on your DBMS and requirements.

Upvotes: 3

Patrick
Patrick

Reputation: 17973

Create a list or BindingSource outside the loop, bind that to your gridview and then add all records to that list or source.

The problem with your current approach is that you are overwriting the records pulled from the database with a new datasource each time, so as you stated, only the last one is "set", and the older assignments are disposed of.

Upvotes: 1

Related Questions