Ayende @ Rahien

Unnatural acts on source code

What is wrong with SortedList

Today it has arouse my ire. Take a look at the IndexOfKey, IndexOfValue and RemoteAt methods.

image

You would expect a list to have a way to get to item by index, certainly based on methods such as RemoveAt and IndexOfXyz.

It doesn't have such a way. You have to violate the Law of Demeter in order to get it to work, list.Values[i], yuck!

Comments

Lucas Goodwin
08/08/2008 10:07 PM by
Lucas Goodwin

I would go so far as to say SortList shouldn't even have integer methods if you are going to use keys for indexing. The parrellel contexts can be... problematic.

Rob
08/09/2008 02:23 AM by
Rob

You couldn't have an int indexer, because someone might want to use integers as keys. Wouldnt adding a "ValueAt(int i)" method just be pollution?

Bryan
08/09/2008 05:25 AM by
Bryan

Maybe not an indexer, but why not a GetAt(int) method? I don't have VS fired up right now, so I don't know what methods are or aren't there, but having one seems reasonable.

Anyway, what really want to know is why there's no pre-canned method for copying the output of one stream to the input of another stream. It's 2008 for God's sake. Let's get a InputStream.CopyTo(OutpuStream) method already.

Frederik Gheysels
08/09/2008 08:26 AM by
Frederik Gheysels

I was pretty sure that the non-generic SortedList did have a method which retrieves the value at a specific index (I used it a few days ago).

So, I checked it right now, and indeed:

The non-generic sortedList has a method 'GetByIndex', whereas the generic SortedList indeed doesn't have this functionality.

Why would they (MS) have discarded it ?

Patrik Hägne
08/11/2008 07:43 AM by
Patrik Hägne

If there was actually an indexer that took an int for the index it should return a KeyValuePair<TKey, TValue> rather than a TValue, so that wouldn't really solve the problem any more elegantly than using the values collection. I guess the solution would be two methods, GetValueAt(int index) and GetKeyAt(int index) but I'm not sure it adds anything but pollution. I believe that the "law" of demeter is something that you should take a very pragmatic approach to rather than regarding it as a law.

Jon Skeet
08/11/2008 09:34 AM by
Jon Skeet

My problem with "SortedList" is that the "list" part is an implementation detail for what is mostly a dictionary in terms of API. I would have preferred them to name it "ListDictionary" with "SortedDictionary" called "TreeDictionary". (Either of those could be prefixed with "Sorted" if desired.)

Daniel Schilling
08/13/2008 12:26 PM by
Daniel Schilling

I'm annoyed that SortedList doesn't implement an interface that describes it's special abilities. I'm thinking something along the lines of:

public interface ISortedList<TKey, TValue>

: IDictionary<TKey, TValue>, IList<TValue>

{

IList<TKey> Keys { get; }

}

The IList<> interface would act in a readonly fashion, throwing NotSupportedException. Any weirdness introduced by the possibility of a SortedList whose keys are ints could be solved as follows:

public class SortedList<TKey, TValue>

: ISortedList<TKey, TValue>

{

....

public TValue this[TKey key]

{

    get { ... }

    set { ... }

}


TValue IList<TValue>.this[int index]

{

    get { ... }

    set { ... }

}

....

}

Having SortedList implement an interface like this would give me a lot more control over some of the NHibernate user types I create without having to do hackish things in my domain objects like accessing PersistentGenericMap.Entries().

Daniel Schilling
08/13/2008 12:51 PM by
Daniel Schilling

Also, add the comparer to the interface:

public interface ISortedList<TKey, TValue>

: IDictionary<TKey, TValue>, IList

{

IList Keys { get; }

IComparer Comparer { get; }

}

Comments have been closed on this topic.