NebulousLabs/Sia — pull request #186 by lukechampine (2014-08-05 21:55:36+00:00)
Title: Reorg

This is by-and-large a cosmetic touch-up of the codebase, intended to satisfy go lint and go vet. Mostly this means adding docstrings, correcting typos, choosing more descriptive names, and generally trying to make the code as idiomatic and godoc-friendly as possible. Some functions have been refactored to be more readable/maintainable without affecting functionality.

**Points of interest**
- Commenting conventions
All exported functions, types, and variables need to have a docstring that begins with the object's name. Unexported objects should have docstrings too, unless they're trivial.
I also noticed that in many of the larger functions, comments are written in full sentences and wrapped to 80 characters. I think this makes sense, though in many places the comments could be slimmed down or obviated entirely when we have a full specification. "Standard comments" should not be written in full sentences, nor wrapped to 80 characters unless they are very long.

- Error conventions:
Errors should not be capitalized or end with punctuation, except perhaps inside test functions. This is because the calling function may want to format the error or surrounded it with context before printing it. For the same reasons, errors generally do not need to supply their own context, as in "foo: unexpected null pointer".
Many instances of `fmt.Errorf()` have been changed to `errors.New()`. The formatting ability of `fmt.Errorf()` is only used 3 times out of 67 total calls, and I think it's confusing to be use a formatting function when formatting is almost never required. It's true that there are some places where a formatted error is desirable, but I think in those errors will eventually be passed to the logger instead of being returned, so `fmt.Errorf()` will be replaced by a logger function. Overall, we should rethink the way we want to handle errors, and consider which errors are valuable to the *program* and which are valuable to the *user* (or developer).

Functionality has been changed in very few places, outlined below. Various smaller bugs have also been fixed.

- `MerkleCollapse()` has been reimplemented recursively, making it much more readable. However, it now takes a `numAtoms` parameter, which broke some existing code. I think specifying the number of atoms to read is probably a good idea anyway. It also returns an error now, instead of just panicking.

- Addresses have been reformatted from {id host port} (the "American way") to {host port id} (the "European way"). Also, `RegisterHandler()` returns a full Address, and RPCServer no longer has an `Address()` method.

- The Reed-Solomon code has been purged of a few bugs, and returns errors now instead of calling `exit(1)`.

- filesCreatedDuringTesting has been removed, in favor of using a Sia folder in the default temp directory (on Linux, /tmp/Sia. To accomplish this, functions have been added to the siafiles package to facilitate creating temporary files. (My next PR will likely be beefing up siafiles.)

Finally, I've added a lot of benchmarks, though some packages could still use more. They can be run with `make bench`.