RSH RSH - 4 months ago 11
C# Question

How to Prevent Extra Loop Iterations?

I want to iterate through

DataReader
three times for scanning the Directory folders.

First Iteration: for all
DataReader
rows with folder directory having folders name starts with P

Second Iteration: for all
DataReader
rows with folder directory having folders name starts with H

Third Iteration: for all
DataReader
rows with scanning for second
column
folder directory having folders name starts with C

C# Code:

Console.WriteLine("Press any key to move Inactive CrsMedia...");
Console.ReadKey();
SqlCommand cmd = new SqlCommand("SELECT h.HotelID,h.ChainID FROM HOTEL h WITH (NOLOCK) WHERE Active=0", con);
SqlDataReader dr = cmd.ExecuteReader();
if (dr.HasRows)
{
while (dr.Read())
{
for (int i = 0; i <= 2; i++)
{
for (int j = 0; j <= dr.FieldCount-1; j++)
{
if (i == 0)
{

chkChar = "P";
Console.WriteLine("CRS Media Moving :{0}", dr["HotelID"].ToString());
MoveDirectory(source + @"\" + chkChar + dr["HotelID"].ToString(), target + @"\" + chkChar + dr["HotelID"].ToString());
}
else if (i == 1)
{
chkChar = "H";
Console.WriteLine("CRS Media Moving :{0}", dr["HotelID"].ToString());
MoveDirectory(source + @"\" + chkChar + dr["HotelID"].ToString(), target + @"\" + chkChar + dr["HotelID"].ToString());
}
else
{
chkChar = "C";
Console.WriteLine("CRS Media Moving :{0}", dr["ChainID"].ToString());
MoveDirectory(source + @"\" + chkChar + dr["ChainID"].ToString(), target + @"\" + chkChar + dr["ChainID"].ToString());
}

}
}
}
}
else
Console.WriteLine("All CrsMedia are In Active state.");


Above code works, but it gives extra messages may due to current iterations are wrong. please check attached image.
enter image description here

What should I need to modify/change in current code?

Answer

Wrong loop condition. It should be:

for (int i = 0; i < 3; i++)

you're iterating 4 times and not 3. from 0 to 3.

Same goes for:

for (int j = 0; j < dr.FieldCount; j++)

Note: The inner loop is useless as you're not using j at all. I think you don't even need the first loop:

while (dr.Read())
{
    chkChar = "P";
    Console.WriteLine("CRS Media Moving :{0}", dr["HotelID"].ToString());
    MoveDirectory(source + @"\" + chkChar + dr["HotelID"].ToString(), target + @"\" + chkChar + dr["HotelID"].ToString());

    chkChar = "H";
    Console.WriteLine("CRS Media Moving :{0}", dr["HotelID"].ToString());
    MoveDirectory(source + @"\" + chkChar + dr["HotelID"].ToString(), target + @"\" + chkChar + dr["HotelID"].ToString());

    chkChar = "C";
    Console.WriteLine("CRS Media Moving :{0}", dr["ChainID"].ToString());
    MoveDirectory(source + @"\" + chkChar + dr["ChainID"].ToString(), target + @"\" + chkChar + dr["ChainID"].ToString());
}

You would better refactor those redundant lines into a separate method and pass to that method the 3 values of chkChar one at a time. Like this:

private void SomeMethodName(IDataReader dataReader, string chkChar, string columnName)
{
    string columnValue = dataReader[columnName].ToString();
    Console.WriteLine("CRS Media Moving :{0}", columnValue);
    MoveDirectory(source + @"\" + chkChar + columnValue, target + @"\" + chkChar + columnValue);
}

And the initial code would be:

while (dr.Read())
{
    SomeMethodName(dr, "P", "HotelID");
    SomeMethodName(dr, "H", "HotelID");
    SomeMethodName(dr, "C", "ChainID");
}