This group of classes and interfaces have a number of issues that make them difficult reason about and test.
Some issues
RepositoryModel, LocalRepositoryModel and RemoteRepositoryModel aren't models at all
- They depend on and hold a reference to
IGitService
LocalRepositoryModel contains mutating methods like Refresh
LocalRepositoryModel contains utility methods like GenerateUrl
Plan of attack
- Move construction of
LocalRepositoryModel into IGitService
- Convert
RepositoryModel and LocalRepositoryModel into pure models
- Remove the interfaces (because interfaces aren't needed for POCO types)
GenerateUrl is only used by LinkCommand so move that there as a private method
Questions
Should the models be mutable or immutable?
@grokys said:
immutable is easier to reason about, but most of our other models are mutable
note that most of our other models are mutable only because the syntax for initializing mutable instances is a lot more readable than for immutable instances
@jcansdale said:
i'd be inclined to keep them mutable for the moment, so we're refactoring less code initially