Editorial: simplify a step in SetValueInBuffer#3819
Editorial: simplify a step in SetValueInBuffer#3819michaelficarra wants to merge 1 commit intomainfrom
Conversation
| 1. Let _execution_ be _AR_.[[CandidateExecution]]. | ||
| 1. Let _eventsRecord_ be the Agent Events Record of _execution_.[[EventsRecords]] whose [[AgentSignifier]] is AgentSignifier(). | ||
| 1. If _isTypedArray_ is *true* and IsNoTearConfiguration(_type_, _order_) is *true*, let _noTear_ be *true*; else let _noTear_ be *false*. | ||
| 1. If _isTypedArray_ is *true*, let _noTear_ be IsNoTearConfiguration(_type_, _order_); else let _noTear_ be *false*. |
There was a problem hiding this comment.
how is this simplified? it's just moving where the "let" happens, and making only one of the possible values be hardcoded, which seems at best the same, and at worst more complex
There was a problem hiding this comment.
I agree with @ljharb: the new version is shorter, but the older one is simpler to read.
There was a problem hiding this comment.
If we wrote this line out as code, it would look like
let noTear = isTypedArray && IsNoTearConfiguration(ty, order) ? true : false;which would be flagged by probably just about any linting configuration. The way we would actually write this in code would probably look something like
let noTear = isTypedArray && IsNoTearConfiguration(ty, order);but the best we can do in spec text is the equivalent of
let noTear = isTypedArray ? IsNoTearConfiguration(ty, order) : false;which definitely seems better than the first example to me.
There was a problem hiding this comment.
Buuuuuut it's not code, it's prose. And as prose, the current text is more readable.
There was a problem hiding this comment.
Yeah keep the original, not a fan of this change
|
🤷♂️ |
5 participants
No description provided.