Cleanup in ApplicationCache

[StanleyGoldman]

As mentioned in #390 (comment)
I changed IRemoteConfigBranchDictionary to implement IDictionary<string,Dictionary<string,ConfigBranch>>

[drguthals]

Why are you implementing using Dictionary instead of IDictionary? Is using the actual class what allows you to take out all of the implementation methods that you also removed?

[StanleyGoldman]

IRemoteConfigBranchDictionary should have never implemented IDictionary<string, IDictionary<string, ConfigBranch>>. Fixing that allowed me to remove a chunk of unused code.

RemoteConfigBranchDictionary inherits Dictionary<string, Dictionary<string, ConfigBranch>> as a result it automatically implements IDictionary<string, Dictionary<string, ConfigBranch>> which is not the same as IDictionary<string, IDictionary<string, ConfigBranch>>.

[shana]

Also, in general we don’t have any custom implementations of IDictionary and other list/collection interfaces, so there’s no advantage in passing interfaces around (tests can use the concrete classes just fine as well). We only pass interfaces around when we need to have them available in GitHub.Api but implement them in GitHub.Unity

Unrelated to Dictionary, just fyi Unity can serialize List<T> so we tend to rely on concrete classes for serializable types.

[StanleyGoldman]

RemoteConfigBranchDictionary has functionality that is specific to Unity (i.e. runs in Mono).
Our unit tests cannot use classes that make reference to Unity's libraries. So those classes are usually represented with interfaces and stubbed in unit tests to make other classes testable.

To aid this effort classes in GitHub.Api in particular don't reference the Unity libraries.

[StanleyGoldman]

IRemoteConfigBranchDictionary lives in GitHub.Api

[jcansdale]

This makes sense. I had questions but they're already been answered above. 👍