Optimizing nested loop for an array within a list [closed]

It would really help if you edited your post to include Bird and Nest classes for context. </rant>

Naming

Before code can be optimized, it needs to be understood. Naming is hard, but good naming is the best way to make your code readable.

Consider calling a cat, a cat bird, a bird:

foreach (Bird bird in birds)
{
    for (int i = 0; i < bird.Nest.Count() - 1; i++)
    {
        if (bird.Nest[i].X == bird.Nest[i + 1].X && bird.Nest[i].Y == bird.Nest[i + 1].Y)
        {
            bird.Nest[i + 1].X = null;
            bird.Nest[i + 1].Y = null;
        }

    }
}

bird is clearer than bs... whatever bs means.

ToList()

var b = birds as List<Bird> ?? birds.ToList();

Given the lack of context, I'm going to make an assumption here, that the method this code is written in is taking a birds parameter, probably of some IEnumerable<Bird> type.

The question is, why do you need a List<Bird> here?

You don't. foreach doesn't need a List<T> to work, and you're not calling any of the List<T>-specific methods - code against interfaces, not implementations. The fact that you "need" to cast to a specific implementation, is a code smell.

I've seen ReSharper recommend this kind of construct when an enumerable was being iterated more than once in a method - which isn't the case here... unless, again, you aren't showing us the whole picture - in which case I'd recommend extracting methods as appropriate, this loop you have here perfectly fits some void ClearAllNests(IEnumerable<Bird>) method.

As for the loop itself, @MarkJohnson's answer uses SelectMany to flatten the nesting, and that's what I would do as well; your if condition becomes a Where condition for each bird's nests.

This is making some assumptions about your code, and I'm not quite sure what you mean by "optimize", but in some cases, the following is an amusing alternative.

var b = birds as List<Bird> ?? birds.ToList();

foreach (var nextNest in b.SelectMany(bird => (
            from nest in bird.Nest as List<BirdNest> ?? bird.Nest.ToList() 
            let nextNest = bird.Nest[bird.Nest.IndexOf(nest) + 1] 
            where (nest.X == nextNest.X && nest.Y == nextNest.Y)
            select nextNest)))
{
    nextNest.X = null;
    nextNest.Y = null;
}

Update: This is basically a re-statement of the original code, making some assumptions. Again, I'm not sure what you're looking to "optimize", but in some sense this is slightly easier to read, given the new naming and the final action. This can be interpreted slightly more easily as: "for each bird's nest that shares the same location as the previous bird's nest, empty out the nest's coordinates".

It is important to specify what is meant by "optimize". When you use that word, you're simply stating "I wanna make it better," which is not very useful and leaves it up to the reader to make assumptions. Is it slow and you need to speed it up? Does it chew up too much memory? Are you looking to make it more readable and thus more maintainable? You can't simply say "yes" to all of the above because a solution for one will often degrade the solution for your other 2 goals.

As noted in the comments, this chunk of code shares the same defect as the original: it won't work because of the way we iterate through the last item.