current community

your communities

more stack exchange communities

Stack Exchange Inbox Reputation and Badges
tour help
Sign up ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.
up vote 6 down vote favorite

I have a block of C# code here that stores data to an ArrayList from the database and compares each with the values from the GridView.

Is there a way to reduce the line of code and improve its readability?

 while (DataReader.Read())
{
    arrHostName.Add(DataReader.GetString(0));
    arrUsers.Add(DataReader.GetString(2));
    arrPSName.Add(DataReader.GetString(3));
}           
foreach (Infragistics.Win.UltraWinGrid.UltraGridRow row in Grid2.Rows)
{
    string rowHostName = row.Cells["HostName"].Value.ToString();
    string rowUsers = row.Cells["Users"].Value.ToString();
    string rowPSName= row.Cells["PS_NAME"].Value.ToString();

    foreach (string a in arrHostName)
    {
        if (rowHostName == a.ToString())
        { 
            row.Appearance.BackColor = Color.Yellow;
            row.Appearance.FontData.Bold = Infragistics.Win.DefaultableBoolean.True;
        }
    }
    foreach (string a in arrUsers)
    {
        if (rowUsers == a.ToString())
        {
            row.Appearance.BackColor = Color.Yellow;
            row.Appearance.FontData.Bold = Infragistics.Win.DefaultableBoolean.True;
        }
    }
    foreach (string a in arrPSName)
    {
        if (rowPSName == a.ToString())
        {
            row.Appearance.BackColor = Color.Yellow;
            row.Appearance.FontData.Bold = Infragistics.Win.DefaultableBoolean.True;
        }
    }
}
share|improve this question

3 Answers 3

up vote 8 down vote accepted

It looks like what you want is ArrayList.Contains.

string rowHostName = row.Cells["HostName"].Value.ToString();
string rowUsers = row.Cells["Users"].Value.ToString();
string rowPSName= row.Cells["PS_NAME"].Value.ToString();

if (arrHostName.Contains(rowHostName) 
    || arrUsers.Contains(rowUsers) 
    || arrPSName.Contains(rowPSName))
{ 
    row.Appearance.BackColor = Color.Yellow;
    row.Appearance.FontData.Bold = Infragistics.Win.DefaultableBoolean.True;
}

And if you only use the collections above to check if they contain some values, you should prefer HashSet over ArrayList.

Of course, you should then change the names of the local variables accordingly, from arrHostName to hostNames and so on. It is always better not to clutter variable names with type information in C#, whose type system is good enough.

share|improve this answer
up vote 1 down vote

for improvement, to reduce no of iterations

    foreach (Infragistics.Win.UltraWinGrid.UltraGridRow row in Grid2.Rows)
    {
        string rowHostName = row.Cells["HostName"].Value.ToString();
        string rowUsers = row.Cells["Users"].Value.ToString();
        string rowPSName= row.Cells["PS_NAME"].Value.ToString();

        foreach (string a in arrHostName)
        {
            if (rowHostName == a)
            { 
                row.Appearance.BackColor = Color.Yellow;
                row.Appearance.FontData.Bold = Infragistics.Win.DefaultableBoolean.True;
 goto Outer;
            }
        }
        foreach (string a in arrUsers)
        {
            if (rowUsers == a)
            {
                row.Appearance.BackColor = Color.Yellow;
                row.Appearance.FontData.Bold = Infragistics.Win.DefaultableBoolean.True;
 goto Outer;
            }
        }
        foreach (string a in arrPSName)
        {
            if (rowPSName == a)
            {
                row.Appearance.BackColor = Color.Yellow;
                row.Appearance.FontData.Bold = Infragistics.Win.DefaultableBoolean.True;
 goto Outer;
            }
        }
 Outer:
                continue;
    }

And if all arraylist having same length, then you could reduce no of for loops

foreach (Infragistics.Win.UltraWinGrid.UltraGridRow row in Grid2.Rows)
        {
            string rowHostName = row.Cells["HostName"].Value.ToString();
            string rowUsers = row.Cells["Users"].Value.ToString();
            string rowPSName= row.Cells["PS_NAME"].Value.ToString();

           for (int i = 0; i < arrHostName.Length; i++)
            {
               if (arrHostName[i].ToString() == rowHostName || arrUsers[i].ToString() == rowUsers || arrPSName[i].ToString() == rowPSName)
                {
                    row.Appearance.BackColor = Color.Yellow;
                    row.Appearance.FontData.Bold = Infragistics.Win.DefaultableBoolean.True;
    break;
                }
            }

        }

Edited 

if (arrHostName[i] == rowHostName || arrUsers[i] == rowUsers || arrPSName[i] == rowPSName)

Updated

foreach (Infragistics.Win.UltraWinGrid.UltraGridRow row in Grid2.Rows)
            {
                string rowHostName = row.Cells["HostName"].Value.ToString();
                string rowUsers = row.Cells["Users"].Value.ToString();
                string rowPSName= row.Cells["PS_NAME"].Value.ToString();

                if(arrHostName.Contains(rowHostName) || arrUsers.Contains(rowUsers) || arrPSName.Contains(rowPSName))
                   {
                        row.Appearance.BackColor = Color.Yellow;
                        row.Appearance.FontData.Bold = Infragistics.Win.DefaultableBoolean.True;
                    }


            }

Further improvement -- reduced more no of lines

foreach (Infragistics.Win.UltraWinGrid.UltraGridRow row in Grid2.Rows)
                {
                    if(arrHostName.Contains(row.Cells["HostName"].Value.ToString()) || arrUsers.Contains(row.Cells["Users"].Value.ToString()) || arrPSName.Contains(row.Cells["PS_NAME"].Value.ToString()))
                       {
                            row.Appearance.BackColor = Color.Yellow;
                            row.Appearance.FontData.Bold = Infragistics.Win.DefaultableBoolean.True;
                       }
                }
share|improve this answer
1  
the reason why I have added goto Outer; is- if you changes the back color of a row in first for, then there is no need to go to other for –  vikas Jun 6 '13 at 6:23
1  
@QKWS length of arraylist are not same right? I mean all having same length or ? –  vikas Jun 6 '13 at 6:31
1  
@QKWS arraylist length would be same as the no of rows coming from datareader right? –  vikas Jun 6 '13 at 6:33
1  
@QKWS edited if logic, so It would work –  vikas Jun 6 '13 at 6:45
1  
use Count instead of length –  vikas Jun 6 '13 at 6:50
up vote 1 down vote

It would be best way to do this.

foreach (Infragistics.Win.UltraWinGrid.UltraGridRow row in Grid2.Rows)
                {
                    if(arrHostName.Contains(row.Cells["HostName"].Value.ToString()) || arrUsers.Contains(row.Cells["Users"].Value.ToString()) || arrPSName.Contains(row.Cells["PS_NAME"].Value.ToString()))
                       {
                            row.Appearance.BackColor = Color.Yellow;
                            row.Appearance.FontData.Bold = Infragistics.Win.DefaultableBoolean.True;
                       }
                }
share|improve this answer

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

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