[Guideline] Where to factor out logic

[tyoshino]

I'd like to write down a clear well-discussed guidance for at which point we factor out logic and for what purpose.

• make it clear which point is defining semantics and which point is left up to implementors of the API
  • Example: ReadableStreamAddReadIntoRequest() (ones listed in the block with the comment "// ReadableStream API exposed for controllers")
• allow optimized invocation with the following logic skipped
  • skip brand checking
    • Example: the if (IsReadableStream(this) === false) block in ReadableStream.getReader()
  • argument validation
    • Example: read(view) with non typed array view
  • argument parsing
    • Example: calling AcquireReadableStreamBYOBReader() directly than calling getReader() (only when the user is sure that it's byte stream)
  • state checking
    • Example: calling ReadableStreamDefaultReaderRead() directly than calling read()
    • Example (no case): ReadableStreamDefaultReaderRead() checks stream._state in itself as it's less common that the user is sure in which state the stream is.
    • Example: calling ReadableStreamDefaultControllerClose() directly than calling close(). The user can know unsolicited closure by listening to cancel() call on the underlying source.
  • result packing
    • Example: calling ReadableStreamTee() directly than calling tee()
• readability and code simplification

[domenic]

So, my guidelines have been mainly:

• Factor out when used more than once to avoid duplication
• Factor out when it might be called by external specs ("General Readable Stream Abstract Operations" plus ReadableStreamDefaultController{Close,Enqueue,Error,GetDesiredSize}, ReadableStreamDefaultReaderRead, etc.)
  • This covers skipping brand checking, argument validation, argument parsing, and state checking largely; we assume other specs preserve the appropriate invariants and so we don't need to check them and throw errors. That is, only author code is potentially "malicious" in this way.

The "Readable Stream Abstract Operations Used by Controllers" separation also drives things somewhat.

Hope this helps?

[tyoshino]

Thanks, Domenic. The main motivation of this was to record reasoning we made for determining listed in your "This covers skipping brand checking, ..." sentence.

I'd like to keep guidelines like this here. Which do you prefer, committing to the repo as an MD file, an open issue, a closed issue, ...

[domenic]

I will work on a Markdown file that encompasses this and other things we've talked about in the past.