0
\$\begingroup\$

This is my current RowDataBound function. I think this functionality is a bit horrrendous really and it winds up with an extreme amount of LINQ SQL queries being passed around (which makes for big server load etc).

What I'm wondering is, is there any way that I could pull all the data in the first select statement, put it somewhere, and then have the fields in the templatefields bind back to this instead of running a query for every single row?

protected void GridView1_RowDataBound(object sender, GridViewRowEventArgs e)
    {

        if (e.Row.RowType == DataControlRowType.DataRow)
        {
            HiddenField hidField = (HiddenField)e.Row.FindControl("dbID");
            Table tbldetails = new Table();
            TableHeaderRow tblheaderrow = new TableHeaderRow();
            TableHeaderCell tblheadercell = new TableHeaderCell();
            tblheadercell.Text = "Details";
            tblheaderrow.Cells.Add(tblheadercell);
            tbldetails.CssClass = "table";
            int rowID = Convert.ToInt32(hidField.Value);
            Panel pnl = (Panel)e.Row.FindControl("deviceDetails");
            bool altField = false;

            var query = (from q in db.GetTable<priceData>()
                         where q.ID == rowID
                         select q).FirstOrDefault();
            var values = typeof(priceData)
                .GetProperties()
                .Select(p => p.GetValue(query, null))
                .ToArray();

            foreach (var value in values)
            {
                TableCell cell = new TableCell();
                TableRow tr = new TableRow();
                cell.Text = value.ToString();
                tr.Cells.Add(cell);
                tbldetails.Rows.Add(tr);
            }
            pnl.Controls.Add(tbldetails);
        }

EDIT

To explain myself a bit better, what I have is a GridView with rows that are expandable. In the expandable row under datarow (which lives in a TemplateField) I want to put all the data from the datarow in the sqldatasource. To me, this means I would have to have a collection of some sort to iterate through, to add to the template field. That said I could be totally wrong and I could be going about this the wrong way. Am I just totally barking up the wrong tree?

\$\endgroup\$
2
  • \$\begingroup\$ Did I phrase this question badly? I think this would be a common situation that people would encounter, surely. \$\endgroup\$
    – Lewis Cianci
    Aug 31 '15 at 4:00
  • \$\begingroup\$ I phrased it badly @BCdotWEB, edited question accordingly. \$\endgroup\$ Sep 1 '15 at 0:34
1
\$\begingroup\$

My main problem with this code is that it feels "rambling". Things happen in the right order, but it's all mangled. For instance:

  • You retrieve hidField on line 6 and then don't use it until line 13.
  • You retrieve pnl on line 14 and then don't do anything with it until line 33.
  • You create tbldetails on line 7 but assign its property CssClass only on line 12 (and "table" is IMHO a bad css class name).

Etcetera. Group these assignments so the code become more logical, so there's more of a flow.


Variable altField isn't used at all.


I'm not a big fan of mixing LINQ styles, and I find the SQL-like version often too verbose. Example:

(from q in db.GetTable<priceData>()
where q.ID == rowID
select q).FirstOrDefault();

... can be rewritten as:

db.GetTable<priceData>().FirstOrDefault(x => x.ID == rowID);

priceData is a class name, so why then is it camelCase instead of PascalCase?


Why not use var instead of repeating the class name like here:

TableHeaderRow tblheaderrow = new TableHeaderRow();
TableHeaderCell tblheadercell = new TableHeaderCell();
Table tbldetails = new Table();

Compound words should have a capital at each compounded word, so it should be tblHeaderRow instead of tblheaderrow etc.


Avoid abbreviations. Why hidField and not hiddenField?


This is "old style" C#:

TableHeaderCell tblheadercell = new TableHeaderCell();
tblheadercell.Text = "Details";

These days I'd expect to see this:

var tableHeaderCell = new TableHeaderCell
{
   Text = "Details"
};

Now, WRT your approach: it depends on the volume of data. If this detail table isn't that "heavy" and the main table isn't that big, I would consider filling it up for each row by default and toggling its visibility through client-side code (JavaScript/JQuery + CSS). Not that in this case you should NOT retrieve it ID by ID, but instead in one query.

If that's too much data to send to the browser in one go, then I'd consider your way. There really is no "one size fits all" solution, it all depends on the volume of data and what your users expect from your UI.

\$\endgroup\$
1
  • \$\begingroup\$ Thank you so much. I've wondered how to do that code in the "these day's I'd expect to see this" part for literally months. Your suggestions will make me a better coder. Thanks again! \$\endgroup\$ Sep 1 '15 at 23:46

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.