6
\$\begingroup\$

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;
 }
 }
}
Jamal
35.2k13 gold badges134 silver badges238 bronze badges
asked Jun 6, 2013 at 6:15
\$\endgroup\$

3 Answers 3

8
\$\begingroup\$

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.

answered Jun 6, 2013 at 7:16
\$\endgroup\$
1
\$\begingroup\$

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;
 }
 } 
answered Jun 6, 2013 at 6:18
\$\endgroup\$
13
  • 1
    \$\begingroup\$ 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 \$\endgroup\$ Commented Jun 6, 2013 at 6:23
  • 1
    \$\begingroup\$ @QKWS length of arraylist are not same right? I mean all having same length or ? \$\endgroup\$ Commented Jun 6, 2013 at 6:31
  • 1
    \$\begingroup\$ @QKWS arraylist length would be same as the no of rows coming from datareader right? \$\endgroup\$ Commented Jun 6, 2013 at 6:33
  • 1
    \$\begingroup\$ @QKWS edited if logic, so It would work \$\endgroup\$ Commented Jun 6, 2013 at 6:45
  • 1
    \$\begingroup\$ use Count instead of length \$\endgroup\$ Commented Jun 6, 2013 at 6:50
1
\$\begingroup\$

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;
 }
 } 
answered Jun 6, 2013 at 18:46
\$\endgroup\$

Your Answer

Draft saved
Draft discarded

Sign up or log in

Sign up using Google
Sign up using Email and Password

Post as a guest

Required, but never shown

Post as a guest

Required, but never shown

By clicking "Post Your Answer", you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.