Skip to content

Instantly share code, notes, and snippets.

@tedsuo
Last active February 2, 2016 18:57
Show Gist options
  • Select an option

  • Save tedsuo/6da49e4b29a88319c249 to your computer and use it in GitHub Desktop.

Select an option

Save tedsuo/6da49e4b29a88319c249 to your computer and use it in GitHub Desktop.
Volman Feedback

volman/cmd/volman/main.go

Don't dot import.

parse flags in init instead of main - not a big deal but a better practice.

passing loggers - logger argument always comes first in every function that takes a logger

exitOnFailure() - use log.Fatal to trigger a log and panic instead

appends are actually prepending currently. Reverse them.

volman/delegate/client_local.go

Don't dot import.

Why is this package called delegate? Admittedly, this is a hard package to name.

volman/handlers

Don't dot import.

Routes should be a public singleton.

Generate() - rename Generate() to New(). Also, this is a RARE case where logger.Fatal can be used to swallow an error.

volman/client.go

Drivers - use the pattern ListDriversRequest{} and ListDriversResponse{} to represent api structs. Go only has structs and interfaces, so a cononical "Model" object is a concept that works poorly in go. It is better to think of every struct as a use-case specific representation, not a canonical representation.

volman/client_remote.go

Don't dot import library and application code in Go.

Merge RemoteClient with RemoteHttpClient. The remote client probably doesn't need it's own package.

RemoteHttpClient_ - Don't use _ in Go. Make attributes private, rather than appending "_"

volman/remote/http_client.go

No need to separate out this library from the remote client. Extract a subobject later into cf_http if it really is shareable code.

Make attributes private, rather than append "_"

Don't use "github.com/errors" or wrap errors at all. Use logging to record the call stack.

RemoteHttpClient - Make concrete implementation private. remoteHttpClient and RemoteHttpClient instead of RemoteHttpClient and RemoteHttpClientInterface.

withAPI - "withAPI" does not descibe this object very well. A single letter "c" or "client" is more correct.

HttpClient interface - this internal to the remote client, and not tested as a dependency, so there is no need for an extra interface here.

Get(fromPath string) RequestReturnInterface - Avoid chaining of functional interfaces, or attempting to chain calls by returning 'self'. None of the benefits of this pattern work in go, and it makes interfaces and mocking more difficult. Favor flatter interfaces that return values.

RequestReturn - To use-specific an object. Unpack this into a cleaner separation of concerns. Use stanard lib objects, such as the json decoder.

AndReturnJsonIn - don't use named return values. You have an error in your code, and the compiler did not catch it because of this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment