Question: Only.Once

[jpillora]

for range Only.Once {

   break
}

I haven't seen this before, what's the benefit over simply returning an error?

[MickMake]

There are several advantages to this.

• Enforces a common return point - which is at the end of the function.
• Unfolds complex AND / OR logic.
• Makes it clear when a bunch of logic can be broken out into it's own function/method.

One of my friends put me on to it several years ago and I've never looked back. It took a while to get used to, but the end result is more stable code.

[jpillora]

Interesting, I have run into these before though I lean in a different direction. For me:

Enforces a common return point - which is at the end of the function.

Instead of aiming to return at the end, I tend to aim to return as early as possible. A function is a tree of code paths. There's one way in at the top (the root), but many ways down (to the leaves). The goal should be to simplify the happy path tree through this tree. Early returns prune branches (e.g. validating input, simple/constant returns, etc). I find this way produces the simplest code.

Unfolds complex AND / OR logic.

Not sure if this is what you mean, though with medium complexity, I usually break up complex AND OR logic into variables:

isFoo := something == 123 && strings.HasPrefix(another, "special")
isBar := oneMore(456) > 789
if isFoo || isBar {
   // do thing
}

I like this better than comments, because you're forced to change the code when it breaks - where comments can go stale.

With high complexity, I usually move it out into a function if isFoo() { ... }

Makes it clear when a bunch of logic can be broken out into it's own function/method.

I do often find myself breaking parts out into their own functions/methods though it seems the difficulty of this task is directly related to Go itself, rather than

if err := foo(); err != nil {
   f.err = err
   break
}
// vs
if err := foo(); err != nil {
   return err
}

Without taking too much of your time, I'm keen to see examples of where you feel your code is improved

[MickMake]

isFoo := something == 123 && strings.HasPrefix(another, "special")
isBar := oneMore(456) > 789
if isFoo || isBar {
   // do thing
}

To rewrite it in the once.only format:

var isFoo bool
for range Only.Once {
    if something == 123 {
        break
    }
    if strings.HasPrefix(another, "special") {
        break
    }
    if oneMore(456) > 789 {
        break
    }
}

var isBar bool
for range Only.Once {
    if oneMore(456) > 789 {
        break
    }
}

if isFoo || isBar {
   // do thing
}

It may look cumbersome, but then it highlights that isFoo and isBar can be moved out into their own functions easily:

func IsFoo(args....) bool {
    var ok bool
    for range Only.Once {
        if something == 123 {
            ok = true
            break
        }
        if strings.HasPrefix(another, "special") {
            ok = true
            break
        }
        if oneMore(456) > 789 {
            ok = true
            break
        }
    }
    return ok
}

func IsBar(args....) bool {
    var ok bool
    for range Only.Once {
        if oneMore(456) > 789 {
            ok = true
            break
        }
    }
    return ok
}

if isFoo() || isBar() {
   // do thing
}

[jpillora]

Thanks for the examples :) To me it does look cumbersome, I guess as someone who's followed the "out-of-the-box" Go style from the start, I find it more difficult to read. In both, I can see that the functionality can be split out, but the Only.Once way has more lines.

I guess if it works for you, then that's good! My last comment would be that this might make others less incentivised to contribute, but that might not be a big factor for you

If I could make one request, it would be to support installing from source with go install github.com/MickMake/GoSungrow@v2.3.0 just work (TM) 😁 It looks like the replace directives enforce a local dependency?

Edit: I'll make it an issue in the other repo :)

[MickMake]

Thanks for that. Yup, I'm in the middle of a bit refactor that reduces a lot of extra coding for each of the endpoints. So, forced the replace in the interim.

IE: I'm using the structures within data.go to tag fields that then get used in formatting to tables, etc. That way, changes to the Sungrow API can be made easier without any coding changes. IE: just the structure.