patterncsharpMinor
Moving List View items from one list to another
Viewed 0 times
viewoneitemsmovinganotherlistfrom
Problem
I'm writing a form that contains 2 ListViews with a set of 4 buttons. 2 move all the items from one to the other in each direction, the other move only selected items in each direction.
Nothing fancy or complicated.
However I'm feeling like I could write the MoveItem method in a more elegant way and was wondering if any would mind taking a look and advising me.
Any suggestions would be thankfully recived
Nothing fancy or complicated.
However I'm feeling like I could write the MoveItem method in a more elegant way and was wondering if any would mind taking a look and advising me.
private void MoveItem(ListView source, ListView dest, bool all)
{
if (all)
{
foreach (ListViewItem item in source.Items)
{
source.Items.Remove(item);
dest.Items.Add(item);
}
}
else
{
foreach (ListViewItem item in source.SelectedItems)
{
source.Items.Remove(item);
dest.Items.Add(item);
}
}
}Any suggestions would be thankfully recived
Solution
Before thinking about how to write this in a more elegant way, you should first ask yourself: is this code actually correct? The “problem” with that code is that you're iterating the collection you're modifying. Quite often (for example in the case of
Now, if you want to refactor code like this, just have a look at what's the same and what's different. You can notice that the only different thing in the two branches is the source of the
List), that's not allowed and your foreach would throw an exception. Fortunately for you, the enumerator of ListViewItemCollection iterates over a snapshot of the collection of the items, so your code will actually work fine (although I didn't find this behavior documented anywhere).Now, if you want to refactor code like this, just have a look at what's the same and what's different. You can notice that the only different thing in the two branches is the source of the
foreach. So, just create a local variable for it, set it correctly and then have common code that iterates it:private static void MoveItem(ListView source, ListView dest, bool all)
{
var items = all ? source.Items : source.SelectedItems;
foreach (ListViewItem item in items)
{
source.Items.Remove(item);
dest.Items.Add(item);
}
}Code Snippets
private static void MoveItem(ListView source, ListView dest, bool all)
{
var items = all ? source.Items : source.SelectedItems;
foreach (ListViewItem item in items)
{
source.Items.Remove(item);
dest.Items.Add(item);
}
}Context
StackExchange Code Review Q#11469, answer score: 5
Revisions (0)
No revisions yet.