Better solution replacing the foreach loop or refining it

Question

this.entityModel.Entities is a source to my datagrid(agdatagrid).

I have kept AutoGenerateColumns="False". i have 6 columns in my agdatgrid

i want 6th column to be visible depending on the data of that column..ie., if any row of that column contains the data then it should be visible and if none of the row contains the data for that column it should be invisible.

So i have written a foreach loop but it takes more time to get ui loaded if the data is large. so is there any other way ?

foreach (BrowserEntity _browseEntity in this.entityModel.Entities)
{
            if (_browseEntity.State != null && this.entityModel.Entities.Count>0)
            {
                this.grid.DataSource = this.entityModel.Entities;
                this.grid.Columns[6].Visible = true;
                break;
            }
            else
            {

                this.grid.DataSource = this.entityModel.Entities;
                this.grid.Columns[6].Visible = false;
            }
}

Answer 1

I'm not a silverlight developer, but why do you check for "this.entityModel.Entities.Count>0" in the foreach loop? I would assume the count is always >0 when you enter the loop, isn't it?

Answer 2

I'm not an expert on this...but why are you resetting the DataSource every time?

bool isColumnVisible = false;
this.grid.DataSource = this.entityModel.Entities;
foreach (BrowserEntity _browseEntity in this.entityModel.Entities)
{
            if (_browseEntity.State != null && this.entityModel.Entities.Count>0)
            {
                isColumnVisible = true;
                break;
            }
}
this.grid.Columns[6].Visible = isColumnVisible;

I think this should be faster...at least I hope so.

Answer 3

First look at the logic of what you're writing. You're checking whether the count of a collection is greater than zero inside a loop that iterates over it; this will always return true as the loop will not run if the collection contains anything. So what you're actually writing is this, when code that either always returns true or which cannot execute is removed:

foreach (BrowserEntity _browseEntity in this.entityModel.Entities)
{
    if (_browseEntity.State != null)
    {
        this.grid.DataSource = this.entityModel.Entities;
        this.grid.Columns[6].Visible = true;
        break;
    }
}

So you're assigning the data source a number of times, and never setting Visible to false, whereas I think what you're actually trying to write is something like this:

// bind the grid but hide column 6
this.grid.DataSource = this.entityModel.Entities;
this.grid.Columns[6].Visible = false;

// if there is any state then show column 6
foreach (BrowserEntity _browseEntity in this.entityModel.Entities)
{
     if (_browseEntity.State != null)
     {
         this.grid.Columns[6].Visible = true;
         break;
     }
}

Alternatively, using Linq, this could be written as the following, which achieves the same thing but is much clearer:

this.grid.DataSource = this.entityModel.Entities;
this.grid.Columns[6].Visible = this.entityModel.Entities.Any(e => e.State != null);