

Show HN: GoWork – A Go Distributed Work Library - ryanskidmore
https://github.com/ryanskidmore/GoWork

======
codo2
To make the code simpler to read, I would suggest removing a bunch of "else
{", like the ones on lines 84, 112 and 119 of main.go. When you have something
like:

if err != nil { return err }

you don't need to put a else after the closing }.

~~~
stickydink
Seriously? That's the critique you have?

Choices like that are 100% personal preference, and belief of which is
"better" again is personal opinion.

Foe the counter argument; I find it far more readable to follow the if/else
blocks, than to rely on identifying which if blocks contain return statements.

Personally, I prefer a step further, keeping all if/else blocks and using a
variable inside the method to track the return value, with a single return
statement at the end of the method.

There are plenty of blogs and articles debating for or against every
permutation of this. But in the end, it really doesn't matter one bit whether
there's an else statement on line X Y and Z.

~~~
tptacek
One reason you're getting downvoted is that the note the parent commenter gave
is pretty squarely in Go idiom. Go favors early-exit, and the Go community is
unusually strict about idiom.

(I'm pretty well acquainted with this particular issue because my C coding
style is strictly anti-early-exit, which drives almost everyone I work with up
a wall. I had to unlearn that habit to write idiomatic Go.)

~~~
stickydink
Understandable, thanks for an explanation

------
StabbyCutyou
I notice you're using interfaces to do your handlers, and (forgive me if i'm
reading this wrong) casting those interfaces to a specific "type" of function
before invoking them.

That seems like it would be inefficient in performance-critical code like a
worker system. Have you considered using typed-functions and function pointers
to accomplish a similar approach?

That said, I could easily be missing something in the design, so feel free to
correct me if I'm wrong.

~~~
ryanskidmore
I've resolved this now, thanks :)

------
nothrabannosir
_> Error is only returned by this function when the secret isn't 32 characters
so as long as your secret is definitely 32 characters you can safely ignore
the err returned by this function._

Personal preference, but in this case I'd advise using panic() instead of
error. Ignoring errors is rarely a good idea: APIs change, other programmers
read their code and don't know about your API, wonder why they ignored the
error, &c. You can now also not change your API to ever actually return
meaningful errors from that function anymore.

Meanwhile, if the failure mode is so well-defined, just put that in the
documentation: "panics if the secret key is not exactly 32 characters long."
Let the user of your library worry about it---they will have to, anyway: if
they don't, they get an error they have to deal with, and suddenly they're
dealing with it after all.

EDIT: From a quick glance, it looks like master gets results back from workers
using .Get(<workid>)? Is it easily possible for master to "select" a bunch of
workers and get the first completed one? If you use (or at least somehow allow
the use of) channels for this, you get all the built-in Go goodness for this
kind of control. Even for dynamic number of channels, the reflect package has
a dynamic .Select that handles it for you. I.e.: provide a channel that I can
<\- work results from, and let Go (and the user of your lib) handle the rest.
Including timeouts :) select { result, ok := <\- workchan: ... ;
<-time.NewTimer(3 * time.Seconds).C : <timeout> }.

~~~
nicksardo
I strongly disagree.

The package author means the error will be nil when the secret is 32
characters long; he doesn't mean you can completely ignore checking it.

Regarding when to use panic: "The convention in the Go libraries is that even
when a package uses panic internally, its external API still presents explicit
error return values." [http://blog.golang.org/defer-panic-and-
recover](http://blog.golang.org/defer-panic-and-recover)

Panics should only be used in cases of programmer error or when the process is
in an unrecoverable state.

~~~
bcgraham
An idea is to include a function in the library called MustNewServer() that
panics instead of returning an error. A usability perk of having single return
is that the function can be chained, i. e.,

    
    
        ws := gowork.MustNewServer(key).AddParams(...)

~~~
jalfresi
As much as I hate chaining functions, I do like the idiom of declaring that a
function may panic with the 'Must' prefix.

As long as there is an alternative function which returns errors :)

------
proveanegative
This looks nice enough as it is but what I think Go needs now is a more direct
OTP clone suitable for new projects. If GoWork grew into that it could very
well eat into Erlang's market share. Many people want their distributed
systems language statically typed and fast. With Erlang you have the prospect
of having to rewrite parts of your applications in C for speed looming
menacingly on the horizon at all times. (Edit: I rewrote the last sentence to
be more clear.)

A disclaimer may be appropriate here: I am aware of the limitations preventing
one from reimplementing the entire OTP in Go. I still think that what you
could get would be "close enough" with a few code guidelines (or a purpose-
built static analyzer).

~~~
ihuman
What do you mean by "OTP"? Do you mean "One Time Password", or is OTP an
abbreviation for something else?

~~~
aleksi
Erlang/OTP:
[http://www.erlang.org/faq/introduction.html](http://www.erlang.org/faq/introduction.html)

------
concernedctzn
Looks good but I always get reminded of this:
[https://twitter.com/rob_pike/status/618357610287226880](https://twitter.com/rob_pike/status/618357610287226880)

~~~
0xdeadbeefbabe
I say keep doing it, because it's a good heuristic for enthusiasm, lines of
code, and perhaps quality.

------
alediaferia
As a little side node I'd personally rename main.go to core.go. Plus, I think
there's room for splitting main.go into more little pieces to ease
maintainability.

