As I mentioned in the last post I did not like that I had to create a new Bookmark when iterating over the Bookmarks. Instead of creating a new Bookmark every time let’s store them in the dictionary. I need to change the declaration of the dictionary to the following:
private Dictionary<string, Bookmark> dictionary = new Dictionary<string, Bookmark>();
When I do this of course the BookmarkCollection code no longer compiles, the Add method, the indexer, and the GetEnumerator method no longer compile-let’s address the Add method first. The Add method does not compile because the value of the dictionary used to be a Uri and now it is of type Bookmark. Here is the updated code:
public void Add(string label, Uri site){ if (label == null || site == null) throw new ArgumentNullException(); Add(new Bookmark(label, site));}
private void Add(Bookmark bookmark){ dictionary[bookmark.Label] = bookmark; }
That compiles, on to the indexer. Just like the Add method the indexer was assuming that the value of the KeyValuePair is a Uri. Here is the fix:
public Uri this[string label]{ get { if (!dictionary.ContainsKey(label)) return null;
return dictionary[label].Uri; } set { Add(label, value); }}
This now compiles as well. The last method to fix is the GetEnumerator. We no longer have to create a new Bookmark each time we can just return the value of the KeyValuePair.
public IEnumerator<Bookmark> GetEnumerator(){ foreach (KeyValuePair<string, Bookmark> bookmark in dictionary) yield return bookmark.Value;}
Now the code compiles and all the tests pass. The refactoring is complete and we did not have to change the interface of the class or any of the tests. I hope this demonstrates some of the power of having a suite of tests. A change like this could have easily introduced a problem that was not detectable. Having the tests, even though they are incomplete, gives us a safety net which allows us to move forward with more confidence.