Proper use of .NET exception

Question

What is the correct exception selected in the following example?

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

List<Song> 

And the method inside the Album is to add the Song :

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

Should I make an exception if a user tries to add an existing song? If so, what type of exception?

I heard the phrase: “Use exceptions only in exceptional circumstances,” but I want to tell the client that he is playing the album exactly what went wrong (not just returning the logical value).

Answer 1

If your use case means that the elements of the collection must be unique, then you should use that data structure to enforce this.

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

However, .NET does not have a separate collection that maintains the sort order, although it is very simple to expand the list to support this.

The approach I used below sacrifices memory for speed, storing unique values ​​in the second HashSet. If the memory size was more important, you just need to do an O (N) check for each add operation. Since the methods are not virtual (for some reason) in the List, I led to hiding the base methods using the new keyword.

Please note that this is just an example, not thread-safe, and should probably not be used in real production applications.

  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]); } } } 

Answer 2

In exactly the same situation, .NET developers at Microsoft decided to throw an ArgumentException with a descriptive message. Oh, and they were pretty consistent in that .

Answer 3

Instead of throwing an exception, you can have an AddSong method that returns a boolean value - true if the song was successfully added, and false otherwise. Personally, I think that throwing an exception would be acceptable in this case, if it would be reasonable to expect that the song is unique in the collection. For example, if a collection is a list of songs on an album, you cannot expect a duplicate song (same name, 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 to be able to throw an exception explaining why the error occurred.

Answer 4

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

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

Answer 5

If you want to offer useful exceptions, you can have a basic exception.

AlbumException

Then create a response from CMerat.

DuplicateSongException

This should, of course, inherit from AlbumException .

Personally, I would make the album class unchanged. In this case, this whole situation will disappear.