[go-nuts] Code review

Nick, thank you for releasing this useful tool as open source. I'll
give it a try later on my raspberryPi.

I have a few style suggestions, but this is not a complete review yet.
Only after writing this email I realized it would have been easier to
add the comments to github directly. I'll do that in the next round of
reviews.

constants like SIZE are better named as Size. This style wasn't
followed in early Go programs, but it's now more common (see the time
package for example)

functions like read_file() should be named as readFile().

variables like read_speed should be named readSpeed.

The "const MB" declaration should be at the package level.

Don't call panic in outputDiff(). Use errors instead.

Calling os.Exit(0) in initialiseStats() is a bit weird. Can't you use
the 'finished' channel instead?

Your doc comments don't follow the usual style. See
http://golang.org/doc/effective_go.html#commentary. Specifically "Doc
comments work best as complete sentences, which allow a wide variety
of automated presentations. The first sentence should be a
one-sentence summary that starts with the name being declared."

Your single-letter flags (w, r, R) are, in my opinion, abusing the
flags system. They are command names, not flags, so they should be
positional arguments. Use flags.Args() to look at positional
arguments. The "go" program command line interface is an excellent
example. A user runs:

go fmt -x <package>

Instead of:

go -fmt -x <package>

Underneath, the go tool commands are pluggable structs with attached
functions, which looks quite nice in code. See:
http://golang.org/src/cmd/go/main.go

This package should help writing command line tools using the go tool
style: http://code.google.com/p/go-commander/. It needs better
documentation, but could be useful already.

I'd have to disagree with you on that. The panic is there to catch a
programmer error rather than anything else - it should never happen.
Yes you are absolutely right. In the original C program they were
single letter commands but I got a bit carried away with the goodness of
the flags package!
OK I'll give it a go.

Thanks for taking the time to do a review and I'll be updating the code
with your points shortly!

Thanks

Nick

That's reasonable. But if stressdisk ever becomes a library, you
should definitely use an error instead of a panic.

What would you do with that error?
How would you handle it?

Potentially, my program could only pass an incorrect input to
stressdisk in corner cases, and maybe my program wants to continue
running if the stressdisk operation failed. Using errors makes this
considerably easier.

Other programmers my think differently ("a panic would make the
problem more obvious"), but in any case, panic in libraries should be
documented ("Passing a and b of different lengths causes a run time
panic").

All this said, a closer inspection of stressdisk shows that this panic
is not likely to happen based on user input and system behavior, but
really only because of a programmer error on stressdisk's side, as
Nick said, so my suggestion is probably not really relevant.