# Remove from list value where date less than max date value

How can I improve this code?

var maxDateValue = list.Where(x => x.ID == 1).Max(c => c.LastUpdDt);
list.RemoveAll(x => x.ID == 1 & x.LastUpdDt!= maxDateValue);

• Could you add a bit of conedt for this code? There is only so much that can be said about 2 lines. Maybe include the method and class where this code is used? – Phrancis Dec 14 '15 at 18:28
• Could you please add more context how you want this code to be improved (e.g. performance, readability, usage, etc.). As is it seems too broad to give you concise answers. – πάντα ῥεῖ Dec 14 '15 at 18:39
• Do not abbreviate unnecessarily: LastUpdDt is no improvement on LastUpdateDate. – BCdotWEB Dec 15 '15 at 8:19

I think that Heslacher's answer skips some important issue because of the wrong arguments for list.RemoveAll:

List<T>.RemoveAll(Predicate<T>) - it takes a predicate, not an IEnumerable. Thus creating a filtered list doesn't make that much sense.

I'll give a bit more verbose code answer.

I'll assume that you operate on class Item.

// Add some code to the Item class
class Item
{
// ...

public bool UpdatedBefore(DateTime time)
{
return LastUpdDt < time;
}
}

// If 1 is widely used in context of Item objects, MagicId and IsMagic should be somewhere else (eg in class Item)
private const int MagicId = 1;

private bool IsMagic(Item item)
{
return item.ID == MagicId;
}

private void RemoveMagicItemsExceptOfMostRecentlyUpdated(List<Item> items)
{
var latestMagicUpdate = items.Where(IsMagic).Max(it => it.LastUpdDt);
Predicate<Item> isNotLatest = item => item.UpdatedBefore(latestMagicUpdate);
items.RemoveAll(it => IsMagic(it) && isNotLatest(it));
}


You may think it is inefficient, because I just generated much more lines of code.

But the benefits are:

• every element I added is well named and gives you its exact meaning
• when you read "RemoveMagicItemsExceptOfMostRecentlyUpdated" and you trust it's fair and does what you say, you don't have to look inside
• Code reuse, consistency, less bugs - next time You would want to write "item.ID == 1", you'll use already existing, well named, describing method IsMagic
• Hi! Welcome to Code Review. Good first answer! – TheCoffeeCup Dec 14 '15 at 22:21

By calculating the entries of the list which satisfy the condition that the ID == 1 which by the way is a magic number which should be avoided, and storing the result in a variable the recalculating isn't needed.

As @JSparrow correctly pointed out in his answer the previous shown code was wrong. Not only takes the RemoveAll() method a predicate as an argument but the used IList<T> interface doesn't have that said method.

So this will do the same

IList<yourType> filteredList = new List<yourType>(list.Where(x => x.ID == 1));
var maxDateValue = filteredList.Max(c => c.LastUpdDt);
list = list.Except(filteredList.Where(x=> x.LastUpdDt != maxDateValue)).ToList();


this eleminates your bug with x.ID == 1 & x.LastUpdDt!= as well. <- single &

Coming back to the magic number/string issue. Why is this 1 and not 88 ? Thats just not clear from the code. If you extract this 1 to a meaningful constant it will be much better.