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;
}
}
}
3 Answers 3
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.
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;
}
}
-
1\$\begingroup\$ the reason why I have added
goto Outer;
is- if you changes the back color of a row in firstfor
, then there is no need to go to otherfor
\$\endgroup\$vikas– vikas2013年06月06日 06:23:50 +00:00Commented Jun 6, 2013 at 6:23 -
1\$\begingroup\$ @QKWS length of arraylist are not same right? I mean all having same length or ? \$\endgroup\$vikas– vikas2013年06月06日 06:31:30 +00:00Commented Jun 6, 2013 at 6:31
-
1\$\begingroup\$ @QKWS arraylist length would be same as the no of rows coming from datareader right? \$\endgroup\$vikas– vikas2013年06月06日 06:33:05 +00:00Commented Jun 6, 2013 at 6:33
-
1\$\begingroup\$ @QKWS edited
if
logic, so It would work \$\endgroup\$vikas– vikas2013年06月06日 06:45:07 +00:00Commented Jun 6, 2013 at 6:45 -
1\$\begingroup\$ use
Count
instead oflength
\$\endgroup\$vikas– vikas2013年06月06日 06:50:26 +00:00Commented Jun 6, 2013 at 6:50
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;
}
}