Can this forloop be rewritten using foreach instead

  • Thread starter Thread starter Tony Johansson
  • Start date Start date
T

Tony Johansson

Hello!
I have this method below that use a for loop to remove a DataRow from a
DataTable.
I have a DataGridView that is named dgvStock
I have a DataTable that is named _dataTable

I just wonder if it's possible to remove the for loop and
use a foreach loop insted but it might be problem because of the index into
dgvStock.

private void DeleteRedRow()
{
//Loop through all DataRow in the DataTable
for(int i = 0; i < _dataTable.Rows.Count; i++)
{
if (dgvStock.Rows.DefaultCellStyle.BackColor ==
System.Drawing.Color.Red)
{
_dataTable.Rows.AcceptChanges();
_dataTable.Rows.Delete();
}
}
}

//Tony
 
Tony Johansson said:
I have this method below that use a for loop to remove a DataRow from a
DataTable.
Click to expand...

It's not allowed in a foreach to alter the members in the collection that
it is enumerating. That means that you cannot use a foreach loop if you use
"Remove" to remove rows from the datatable. However, you are using "Delete",
which does not remove the row, but merely marks it as deleted. Therefore,
your loop should be fine if you change it into foreach. Unfortunately, this
means that you wouldn't have the "i" index into the DataGridView. You can
still declare an "int i" and increment it inside the foreach loop, but if
you are going to do this, you might as well leave the "for" as it is.

By the way, there is a problem with your code that can make it not work
as expected if there is more than one red row. You are calling AcceptChanges
on each iteration, so only the last row would remain maked as "should be
deleted" in the datatable. If you are then sending the changes to the server
with a dataadapter, only the last red row will be deleted. Also, the
AcceptChanges would alter the rows in the DataTable, which can cause the
loop to not work as expected, or to fail if you change it into a foreach. I
suggest moving the AcceptChanges outside of the loop, and only calling it
after you have called Update on the DataAdapter.
 
Tony Johansson said:
I have this method below that use a for loop to remove a DataRow from a
DataTable.
Click to expand...

It's not allowed in a foreach to alter the members in the collection that
it is enumerating. That means that you cannot use a foreach loop if you use
"Remove" to remove rows from the datatable. However, you are using "Delete",
which does not remove the row, but merely marks it as deleted. Therefore,
your loop should be fine if you change it into foreach. Unfortunately, this
means that you wouldn't have the "i" index into the DataGridView. You can
still declare an "int i" and increment it inside the foreach loop, but if
you are going to do this, you might as well leave the "for" as it is.

By the way, there is a problem with your code that can make it not work
as expected if there is more than one red row. You are calling AcceptChanges
on each iteration, so only the last row would remain maked as "should be
deleted" in the datatable. If you are then sending the changes to the server
with a dataadapter, only the last red row will be deleted. Also, the
AcceptChanges would alter the rows in the DataTable, which can cause the
loop to not work as expected, or to fail if you change it into a foreach. I
suggest moving the AcceptChanges outside of the loop, and only calling it
after you have called Update on the DataAdapter.
 
I just wonder if it's possible to remove the for loop and
use a foreach loop insted but it might be problem because of the index
into dgvStock.
Click to expand...

This is the least of your problems. You should first do some
reading/experimentation to understand how "AcceptChanges()" and "Delete()"
work. This might impact the very collection you're iterating, as Alberto
pointed out (and likely others to follow). Your logic needs to be re-worked
not just from this perspective however. First, it assumes that your table
and the grid are always the same size (presumably) and that the index of
each row in the collection matches its index in the grid. This is a very
brittle approach and asking for trouble. So is relying on the color.
Assuming there's no native way to check this (whatever it is your color is
signifying), a dedicated flag is more appropriate and much safer. A typical
way to handle this is to rely on the "DataGridViewRow.Tag" member. You can
assign it your own home-grown object which stores information about each row
(such as your flag). Alternatively, though there's probably more work
involved, a "DataGridViewRow" derivative would be even cleaner in theory
(you can add your flag here). In either case you can also store your data
table record as well, either directly by storing its reference, or
indirectly by using its DB key or maybe even its index into the collection
itself, assuming this never changes (caution should be exercised). You'll
need to evaluate what's best for your needs. In any case, you can then use
"foreach" to iterate the grid itself, checking the flag which is now stored
in each row (instead of the color), and invoking "Delete()" on the record
you've also stored in each row.
 
I just wonder if it's possible to remove the for loop and
use a foreach loop insted but it might be problem because of the index
into dgvStock.
Click to expand...

This is the least of your problems. You should first do some
reading/experimentation to understand how "AcceptChanges()" and "Delete()"
work. This might impact the very collection you're iterating, as Alberto
pointed out (and likely others to follow). Your logic needs to be re-worked
not just from this perspective however. First, it assumes that your table
and the grid are always the same size (presumably) and that the index of
each row in the collection matches its index in the grid. This is a very
brittle approach and asking for trouble. So is relying on the color.
Assuming there's no native way to check this (whatever it is your color is
signifying), a dedicated flag is more appropriate and much safer. A typical
way to handle this is to rely on the "DataGridViewRow.Tag" member. You can
assign it your own home-grown object which stores information about each row
(such as your flag). Alternatively, though there's probably more work
involved, a "DataGridViewRow" derivative would be even cleaner in theory
(you can add your flag here). In either case you can also store your data
table record as well, either directly by storing its reference, or
indirectly by using its DB key or maybe even its index into the collection
itself, assuming this never changes (caution should be exercised). You'll
need to evaluate what's best for your needs. In any case, you can then use
"foreach" to iterate the grid itself, checking the flag which is now stored
in each row (instead of the color), and invoking "Delete()" on the record
you've also stored in each row.
 
Tony,

I wrote to you that this would not work if you did it direct on the physic
datatable.
The defaultview is for that, what sense does it make to help you?

Beside as the others wrote the acceptchanges.
A delete is not a remove by the way, but that is as well told.

Cor
 
Tony,

I wrote to you that this would not work if you did it direct on the physic
datatable.
The defaultview is for that, what sense does it make to help you?

Beside as the others wrote the acceptchanges.
A delete is not a remove by the way, but that is as well told.

Cor
 
Hello!

If I change the DataGridView in this way which is the first row in the
DataGridView
dgvStock.Rows[0].Tag = "DEL";

How does it come that this GetChanges() is returning null I mean that I have
changed the DataGridView that is connected to the DataTable so even the
DataTable would recognize this as a change and not retun null for
GetChanges()
if (_dataTable.GetChanges() == null)
return;

//Tony


Cor Ligthert said:
Tony,

I wrote to you that this would not work if you did it direct on the physic
datatable.
The defaultview is for that, what sense does it make to help you?

Beside as the others wrote the acceptchanges.
A delete is not a remove by the way, but that is as well told.

Cor

Tony Johansson said:
Hello!
I have this method below that use a for loop to remove a DataRow from a
DataTable.
I have a DataGridView that is named dgvStock
I have a DataTable that is named _dataTable

I just wonder if it's possible to remove the for loop and
use a foreach loop insted but it might be problem because of the index
into dgvStock.

private void DeleteRedRow()
{
//Loop through all DataRow in the DataTable
for(int i = 0; i < _dataTable.Rows.Count; i++)
{
if (dgvStock.Rows.DefaultCellStyle.BackColor ==
System.Drawing.Color.Red)
{
_dataTable.Rows.AcceptChanges();
_dataTable.Rows.Delete();
}
}
}

//Tony
Click to expand...
Click to expand...
 
Hello!

If I change the DataGridView in this way which is the first row in the
DataGridView
dgvStock.Rows[0].Tag = "DEL";

How does it come that this GetChanges() is returning null I mean that I have
changed the DataGridView that is connected to the DataTable so even the
DataTable would recognize this as a change and not retun null for
GetChanges()
if (_dataTable.GetChanges() == null)
return;

//Tony


Cor Ligthert said:
Tony,

I wrote to you that this would not work if you did it direct on the physic
datatable.
The defaultview is for that, what sense does it make to help you?

Beside as the others wrote the acceptchanges.
A delete is not a remove by the way, but that is as well told.

Cor

Tony Johansson said:
Hello!
I have this method below that use a for loop to remove a DataRow from a
DataTable.
I have a DataGridView that is named dgvStock
I have a DataTable that is named _dataTable

I just wonder if it's possible to remove the for loop and
use a foreach loop insted but it might be problem because of the index
into dgvStock.

private void DeleteRedRow()
{
//Loop through all DataRow in the DataTable
for(int i = 0; i < _dataTable.Rows.Count; i++)
{
if (dgvStock.Rows.DefaultCellStyle.BackColor ==
System.Drawing.Color.Red)
{
_dataTable.Rows.AcceptChanges();
_dataTable.Rows.Delete();
}
}
}

//Tony
Click to expand...
Click to expand...
 
Hello!

If I change the DataGridView in this way which is the first row in the
DataGridView
dgvStock.Rows[0].Tag = "DEL";

How does it come that this GetChanges() is returning null I mean that I have
changed the DataGridView that is connected to the DataTable so even the
DataTable would recognize this as a change and not retun null for
GetChanges()
if (_dataTable.GetChanges() == null)
return;

//Tony


Cor Ligthert said:
Tony,

I wrote to you that this would not work if you did it direct on the physic
datatable.
The defaultview is for that, what sense does it make to help you?

Beside as the others wrote the acceptchanges.
A delete is not a remove by the way, but that is as well told.

Cor

Tony Johansson said:
Hello!
I have this method below that use a for loop to remove a DataRow from a
DataTable.
I have a DataGridView that is named dgvStock
I have a DataTable that is named _dataTable

I just wonder if it's possible to remove the for loop and
use a foreach loop insted but it might be problem because of the index
into dgvStock.

private void DeleteRedRow()
{
//Loop through all DataRow in the DataTable
for(int i = 0; i < _dataTable.Rows.Count; i++)
{
if (dgvStock.Rows.DefaultCellStyle.BackColor ==
System.Drawing.Color.Red)
{
_dataTable.Rows.AcceptChanges();
_dataTable.Rows.Delete();
}
}
}

//Tony
Click to expand...
Click to expand...
 
Hello!

If I change the DataGridView in this way which is the first row in the
DataGridView
dgvStock.Rows[0].Tag = "DEL";

How does it come that this GetChanges() is returning null I mean that I have
changed the DataGridView that is connected to the DataTable so even the
DataTable would recognize this as a change and not retun null for
GetChanges()
if (_dataTable.GetChanges() == null)
return;

//Tony


Cor Ligthert said:
Tony,

I wrote to you that this would not work if you did it direct on the physic
datatable.
The defaultview is for that, what sense does it make to help you?

Beside as the others wrote the acceptchanges.
A delete is not a remove by the way, but that is as well told.

Cor

Tony Johansson said:
Hello!
I have this method below that use a for loop to remove a DataRow from a
DataTable.
I have a DataGridView that is named dgvStock
I have a DataTable that is named _dataTable

I just wonder if it's possible to remove the for loop and
use a foreach loop insted but it might be problem because of the index
into dgvStock.

private void DeleteRedRow()
{
//Loop through all DataRow in the DataTable
for(int i = 0; i < _dataTable.Rows.Count; i++)
{
if (dgvStock.Rows.DefaultCellStyle.BackColor ==
System.Drawing.Color.Red)
{
_dataTable.Rows.AcceptChanges();
_dataTable.Rows.Delete();
}
}
}

//Tony
Click to expand...
Click to expand...
 
Tony Johansson skrev:
Hello!

If I change the DataGridView in this way which is the first row in the
DataGridView
dgvStock.Rows[0].Tag = "DEL";

How does it come that this GetChanges() is returning null I mean that I have
changed the DataGridView that is connected to the DataTable so even the
DataTable would recognize this as a change and not retun null for
GetChanges()
if (_dataTable.GetChanges() == null)
return;
Click to expand...

The Tag element has nothing to do with your your data table, so you
don't have any changes.
 
Tony Johansson skrev:
Hello!

If I change the DataGridView in this way which is the first row in the
DataGridView
dgvStock.Rows[0].Tag = "DEL";

How does it come that this GetChanges() is returning null I mean that I have
changed the DataGridView that is connected to the DataTable so even the
DataTable would recognize this as a change and not retun null for
GetChanges()
if (_dataTable.GetChanges() == null)
return;
Click to expand...

The Tag element has nothing to do with your your data table, so you
don't have any changes.
 
Back
Top