kellabyte/atlas — pull request #1 by benbjohnson (2013-11-09 21:48:49+00:00)
Title: Code review.

I'll add some line notes but here's a couple overall tips:

1. Using interfaces helps a lot as your project grows. It allows packages to communicate without having dependencies. It also allows you to mock types during testing. And finally it lets you really control what the exact API looks like and separates it more from the implementation.

2. I tend to make most types with method (i.e. types that do more than just store data) accessible through interfaces and make the type itself unexported. That makes it necessary to make `NewXXX()` functions which are nice since it ensures that your types will always be initialized.

3. Since `NewXXX()` ensures your types are initialized, you don't need to check for nil on the caller or on any internal slices or maps.

4. Go style is to have short names. Caller objects are usually a single character. I also took out `Topology` from `RingTopology` since it was implied by the package name. From other packages it would be referenced as `topology.Ring`. Also, I usually use `s` for slice for slice types implemented for the `sort` package (e.g. `Nodes`).

5. I added a top-level `Topology` interface that `Ring` extends. `AddNode()` would probably be a good function to promote up to it. Maybe the replication factor stuff too.

7. I removed the `DataCenter` from the `ReplicationFactor` methods since it seemed implied. Although maybe it's better to have a longer name if there are other things that have replication factors too. I would probably name it `DataCenterReplicationFactor` instead of `ReplicationFactorForDataCenter` though.

6. I use @stretchr's testify library for assertions and mocks. As much as I love the simplicity of Go, not having a simple assert is a pain in the ass.