Correct Use of .NET Exception

What is the correct exception to throw in the following instance?

If, for example, I have a class: Album with a collection of Songs:

List<Song>

And a method within Album to add a Song:

public void AddSong(Song song)
{
    songs.Add(song);
}

Should I throw an exception if a user attempts to add a song that already exists? If so, what type of exception?

I have heard the phrase: "Only use exceptions in exceptional circumstances", but I want to tell the client implementing Album exactly what has gone wrong (not just return a Boolean value).

In exactly the same situation, the .NET designers at Microsoft chose to throw ArgumentException with a descriptive message. Oh, and they were pretty consistent about that.

If your use case implies that items in the collection should be unique, then you should use a datastructure that enforces that.

By doing that, you not only avoid having to write a O(N) lookup method to check for duplicates, but you can also bubble up the pre-existing duplicate key exception that a collection of this sort would throw.

However, .NET does not have a distinct collection that preserves sort order, though it is very easy to extend List to support this.

The approach I used below sacrifices memory footprint for speed, by storing the unique values in a second HashSet. If memory size was more important, you'd just have to do a O(N) check on each Add operation. Because methods are not virtual (for some reason) in List, I resulted to hiding the base methods using the new keyword.

Note that this is just an example, and is not thread safe, and should probably not be used in a real production application.

    public class UniqueList<T> : List<T>
    {
        private HashSet<T> _internalHash = new HashSet<T>();

        public UniqueList() : base() { }
        public UniqueList(IEnumerable<T> collection) : base(collection) { }
        public UniqueList(int capacity) : base(capacity) { }

        public new void Add(T item)
        {
            if (!_internalHash.Add(item))
                throw new ArgumentException("Item already exists in UniqueList");

            base.Add(item);
        }

        public new void AddRange(IEnumerable<T> collection)
        {
            foreach (T t in collection)
            {
               this.Add(t);
            }
        }

        public new bool Remove(T item)
        {
            _internalHash.Remove(item);
            return base.Remove(item);               
        }

        public new int RemoveAll(Predicate<T> match)
        {
            int removedElems = 0;

            foreach (T item in this)
            {
                if (match(item))
                {
                    this.Remove(item);
                    removedElems++;
                }
            }

            return removedElems;
        }

        public new void RemoveAt(int index)
        {                
           this.Remove(this[index]);             
        }

        public new void RemoveRange(int index, int count)
        {
            for (int i = index; i < count; i++)
            {
                this.Remove(this[i]);
            }
        }
    }

Instead of throwing an exception you could have the AddSong method return a boolean - true if the song is successfully added and false otherwise. Personally, I think throwing an exception would be acceptable in this case if it's reasonable to expect that the song is unique in the collection. For an example, if the collection is a list of songs on an album you don't reasonable expect a duplicate song (same title, same duration, same position in the sequence of tracks, etc.). You have the option of creating your own exception class derived from System.Exception to create custom errors if you want so you could throw an exception that explains exactly why the error occurred.

You can always create your own exceptions. Simply create a class that inherits from Exception(or, in this case, ArgumentException).

Something along the lines of DuplicateItemException (or DuplicateSongException if you want something very specific) sounds about right.

If you want to offer up useful exceptions you may want to have a base exception.

AlbumException

Then taking from CMerat's answer create.

DuplicateSongException

This should of course inherit from AlbumException.

Personally, I would make the Album class immutable. In that case this whole situation would disappear.