[maccard]

This is a great post, and I agree with much of what he said (range shouldn't copy - I would love a range that iterates by const-reference by default, to lift a phrase from C++).

Deterministic select I hard disagree with. The code in the blog post is race-y, and needs to be fixed, not select. If anything, making select deterministic will introduce _more_ subtle bugs when developers rely on that behavior only to find out in the real world that things aren't necessarily as quick as they are in development.

[sethvargo]

How would you fix that code?

[assbuttbuttass]

I would write the author's example as follows:

    for ctx.Err() == nil {
        select {
        case <-ctx.Done():
            return nil
        case thing := <-thingCh:
            // Process thing...
        case <-time.After(5*time.Second):
            return errors.New("timeout")
        }
    }

The extra check for ctx.Err before the select statement easily resolves the author's issue.

[sethvargo]

It really doesn't though. It handles the case where the context might have expired or be cancelled, but there's still a race when entering the select between the ctx.Done() and reading from thingCh. You may end up processing one additional unit of work. In situations where the exit condition is channel-based, this won't work.

Additionally, this would only work if you had one predominant condition and that condition was context-based. If you have multiple ordered conditions upon which you want to exit, I can't think of how you'd express that as a range.

[assbuttbuttass]

I'm not sure what you mean. There's always going to be a race condition between ctx.Done and thingCh, just depending on whether there's data available. This race condition is unavoidable.

I guess you're thinking of "what if thingCh and ctx.Done activate simultaneously?"

There's no real difference between happening simultaneously and happening one after another.

As for your other point, you can just write code like

    select {
    case x := <-conditionA:
        return x
    default:
    }
    select {
    case x := <-conditionB:
        return x
    default:
    }
    ...

But I've personally never needed code like this.

[sethvargo]

Also, this isn't semantically correct. In order to ensure that `conditionaA` is _always_ preferred over `conditionB`, you must also check if `conditionA` has received a value inside of `conditionB`:

    select {
    case a := <-conditionA:
        return a
    default:
    }
    select {
    case b := <-conditionB:
      case a := <-conditionA:
          return a
      default:

      return b
    default:
    }

[assbuttbuttass]

It would be easier to discuss with a more concrete example. If I ever had to write code like the above I would reconsider the design and try to come up with something simpler.

[sethvargo]

Right, which is noted in the post. That verbosity is, well, verbose. I generally need this in 20% of things I write.

[oconnor663]

In this use case, is it bad if the Done signal arrives the instant after you check it?

[morelisp]

Context cancellation propagates (potentially) asynchronously anyway, so if you're relying on something canceling your context and that immediately appearing you already have a bug.

I've written `select { ..., default: }` enough times I also wish it had shorthand syntax - sometimes it's even clearer to range one "primary" channel and lead the code block with that check - but I cannot think of a case where relying on a deterministic select would not have led to a bug.

[schrodinger]

I think you could do which I'd argue is more idiomatic (

    for {
      if _, ok := <- doneCh; ok {
        break
      }
      select {
      case thing := <-thingCh:
        // ... long-running operation
      case <-time.After(5*time.Second):
        return fmt.Errorf("timeout")
      }
    }

Which goes along w/ https://github.com/golang/go/wiki/CodeReviewComments#indent-... of "Indent error flow".

edit: nvm, your break would be blocked until one of the other channels produced a value. you'd need to check for the doneCh redundantly again in the select.