How to improve compare of item in sorted List<MyItem> with item before and after current item?
Does anyone know about a good way to accomplish this task?
Currently i'm doing it more ore less this way, but i'm feeling someway unhappy with this code, unable to say what i could immediately improve.
So if anyone has a smarter way of doing this job i would be happy to know.
private bool Check(List<MyItem> list)
{
bool result = true;
//MyItem implements IComparable<MyItem>
list.Sort();
for (int pos = 0; pos < list.Count - 1; pos++)
{
bool previousCheckOk = true;
if (pos != 0)
{
if (!CheckCollisionWithPrevious(pos))
{
MarkAsFailed(pos);
result = false;
previousCheckOk = false;
}
else
{
MarkAsGood(pos);
}
}
if (previousCheckOk && pos != list.Count - 1)
{
if (!CheckCollisionWithFollowing(pos))
{
MarkAsFailed(pos);
result = false;
}
else
{
MarkAsGood(pos);
}
}
}
return result;
}
private bool CheckCollisionWithPrevious(int pos)
{
bool checkOk = false;
var previousItem = _Item[pos - 1];
// Doing some checks ...
return checkOk;
}
private bool CheckCollisionWithFollowing(int pos)
{
bool checkOk = false;
var followingItem = _Item[pos + 1];
// Doing some checks ...
return checkOk;
}
Update
After reading the answer from Aaronaught and a little weekend to refill full mind power i came up with the following solution, that looks far better now (and nearly the same i got from Aaronaught):
public bool Check(DataGridView dataGridView)
{
bool result = true;
_Items.Sort();
for (int pos = 1; pos < _Items.Count; pos++)
{
var previousItem = _Items开发者_高级运维[pos - 1];
var currentItem = _Items[pos];
if (previousItem.CollidesWith(currentItem))
{
dataGridView.Rows[pos].ErrorText = "Offset collides with item named " + previousItem.Label;
result = false;
sb.AppendLine("Line " + pos);
}
}
dataGridView.Refresh();
return result;
}
It's certainly possible to reduce the repetition:
private bool Check(List<MyItem> list)
{
list.Sort();
for (int pos = 1; pos < list.Count; pos++)
{
if (!CheckCollisionWithPrevious(list, pos))
{
MarkAsFailed();
return false;
}
MarkAsGood();
}
return true;
}
private bool CheckCollisionWithPrevious(List<MyItem> list, int pos)
{
bool checkOk = false;
var previousItem = list[pos - 1];
// Doing some checks ...
return checkOk;
}
Assuming that CheckCollisionWithPrevious
and CheckCollisionWithFollowing
perform essentially the same comparisons, then this will perform the same function with a lot less code.
I've also added the list
as a parameter to the second function; it doesn't make sense to be taking it as a parameter in the first function, but then referencing a hard-coded member in the function it calls. If you're going to take a parameter, then pass that parameter down the chain.
As far as performance is concerned, though, you're re-sorting the list every time this happens; if it happens often enough, you might be better off using a sorted collection to begin with.
Edit: And just for good measure, if the whole point of this code is just to check for some kind of duplicate key, then you would be way better off using a data structure that prevents this in the first place, such as a Dictionary<TKey, TValue>
.
精彩评论