What is wrong with the following code and what would you do to fix or improve the code if applicable? private void RemoveDuplicates( List<int> zMyList ) { int listCount = zMyList.Count; for( int i = 0 ; i < listCount; i++ ) { for( int j = i + 1; j < listCount ; j++ ) { if( zMyList[i] == zMyList[j] ) { zMyList.RemoveAt(j); } } } }
Sigiloso
When an element is removed, you will get an out of bounds error. To solve, you could move ‘count’ but I would not as it will be more memory intensive. I would just decrement from ‘count’ instead of incrementing so that when you remove an element, you won’t get an out of bounds. If you wanted to use LINQ, you could use zMyList.Distinct(); to get a collection of unique values. As there is no null checking, you could return zMyList == null ? Enumerable.Empty : zMyList.Distinct(); to return an empty collection if it’s null, else use distinct values. Also, it shouldn’t be a void because it’s manipulating the inbound collection which is a code smell. It should return the filtered collection rather than editing the original.