Notably, the block in lines 15-17 was quite interesting.
At first, I left it as:
if response[0] != cmd_foo1[0] {
t.hardReset()
return fmt.Errorf("invalid response to FOO1 [% 02X]", response)
}
At this point, the t.hardReset()
fragment was somewhat standing out in the flow of code,
feeling like something that I could easily forget to call when writing the code from scratch.
But then I remembered of seeing a check err
pattern used somewhere in the design draft,
and realized I could use this here.
The resulting code is even more surprising.
I like that it introduces some reuse,
and hides the t.hardReset()
as a "default behavior".
At the same time, I dislike two things about it:
- that the block is missing an explicit
return
,
so I don't easily see it's a 100% sure exit point from the function;
- that the line seems to read weirdly in English:
check Error("some message")
?...
there's nothing to check here,
there's just an error to return
(and t.hardReset()
to be called).
(That said, try fmt.Errorf(...)
would have the same issue.)
Personally, I'm not 100% sure what I think of the change.
Certainly, the code looks shorter and a bit more terse;
no more "if err != nil" boilerplate.
The block doing additional (substantial) verification in l.15-17 now stands out more,
which seems a good thing.
On the other hand, the "handle" block looks a bit weird.
Now that I think of it,
it somehow feels to be missing symmetry with defer blocks syntactically —
while being most similar to defer blocks semantically
(more so than to if
, switch
, for
, etc.
— as "handle" is not immediate as these are).
Also, I don't like that it feels like I have to do a bit more jumping around in the code with my eyes,
and/or carry more context in memory when reading and analyzing it
(namely, contents of the handle
block).
But again on a positive note,
I can worry less if I haven't made a typo in one of the "if err != nil" blocks.
Any outstanding behaviors from the default handle
contents would have to be made explicit now.
In your original code, if you name your return value, after line 8, you can have:
At this point you don't need a handle at all. Named return values and defer are very powerful. I then see
check
as a way to eliminate, err
andif err != nil { return err }
from the code, and nothing more.