Path: csiph.com!eternal-september.org!reader02.eternal-september.org!.POSTED!not-for-mail From: Ben Bacarisse Newsgroups: comp.lang.javascript Subject: Re: Understanding && Date: Mon, 19 Apr 2021 23:08:41 +0100 Organization: A noiseless patient Spider Lines: 173 Message-ID: <87lf9eyljq.fsf@bsb.me.uk> References: <87wnt2e06e.fsf@bsb.me.uk> <87mttydm95.fsf@bsb.me.uk> <87o8edd8qk.fsf@bsb.me.uk> <87zgxwa5sg.fsf@bsb.me.uk> <87mttv42sv.fsf@bsb.me.uk> Mime-Version: 1.0 Content-Type: text/plain Injection-Info: reader02.eternal-september.org; posting-host="97eff4e9a810452bcd6ea147a93a2f24"; logging-data="28442"; mail-complaints-to="abuse@eternal-september.org"; posting-account="U2FsdGVkX19FzfLDZBMUfEcrwarE875GD4wWVdYOmrY=" Cancel-Lock: sha1:HxeFYaM3ul3wXDxWLWPjMZAAxAI= sha1:+QQs+3Y/a/Av2MgJiV58SZxLB74= X-BSB-Auth: 1.35c432029ddd42436f38.20210419230841BST.87lf9eyljq.fsf@bsb.me.uk Xref: csiph.com comp.lang.javascript:39988 Jon Ribbens writes: > On 2021-04-18, Ben Bacarisse wrote: >> Jon Ribbens writes: >>> but almost always I will end up using instead: >>> >>> if (complex_expression) { >>> complex_conditional >>> } >>> >>> because it is probably easier for others to read ("others" perhaps >>> including me a year later...) And by "easier" I don't mean incompetent >>> programmers can't understand it, I just mean it will take most people >>> slightly longer to understand / have a slightly higher percentage chance >>> of being misunderstood. >> >> That's another interesting perspective. When I look back on old code, >> it's never things like this that take any time at all. I often wish >> there were a large block comment explaining the algorithm or method >> being used, and sometimes I just wish for a comment explaining /why/ >> count is being set to i + j, but I never lose even a moment over /what/ >> the code is doing (in the sense of what the syntax means). > > Here's a topical conundrum I have right now about readability... > I needed to implement CSS.escape, which node doesn't have built-in: > > https://drafts.csswg.org/cssom/#the-css.escape%28%29-method > > Two implementations I have written are shown below. They both do exactly > the same thing. I'd say the first one is *much* more readable. But sadly > it's almost *five times* slower than the second one. So the dilemma is > which one to use... How important is the speed? Five times almost nothing is almost nothing, so one would have to know how much this code is used and how slow the slow one is. But it's an excellent example of what we've been talking about. I found your preferred version (the first one) quite hard to verify. My difficulty was not due to any language feature that you'd used but came from the algorithm combining a negative character class and some positive tests to implement a set of rules that are not written that way. > CSS.escape functions: > > function escape1 (value) { > const string = String(value) > if (string === '-') return '\\-' > return string.replace(/[^-_a-zA-Z\u0080-\uffff]/g, (char, offset) => { > if (char === '\0') return '\ufffd' > const digit = /[0-9]/.test(char) > if (digit && (offset > 1 || !/^[0-9-]/.test(string))) return char > if (char >= ' ' && char !== '\x7f' && !digit) return '\\' + char > return '\\' + char.charCodeAt(0).toString(16) + ' ' > }) > } > > function escape2 (value) { > const string = String(value) > if (string === '-') return '\\-' > let output = '' > for (let offset = 0; offset < string.length; offset++) { > let char = string[offset] > if (char === '-' || char === '_' || (char >= 'a' && char <= 'z') || > (char >= 'A' && char <= 'Z') || char >= '\x80' || > ((char >= '0' && char <= '9') && (offset > 1 || (string[0] !== '-' && > (string[0] < '0' || string[0] > '9'))))) { > } else if (char === '\0') { > char = '\ufffd' > } else if (char >= ' ' && char !== '\x7f' && (char < '0' || char > '9')) { > char = '\\' + char > } else { > char = '\\' + char.charCodeAt(0).toString(16) + ' ' > } > output += char > } > return output > } For me, readability is about how easily can I see that the code is doing the right thing, and given the specification: https://drafts.csswg.org/cssom/#serialize-an-identifier I would write something like this: function escape3 (value) { const string = String(value) return string.replace(/./g, (c, index) => { if (c === '\0') return '\ufffd' if (inRange(c, '\1', '\1f')) return escapedAsCodePoint(c) if (index === 0 && inRange(c, '0', '9')) return escapedAsCodePoint(c) if (index === 1 && inRange(c, '0', '9') && string[0] === '-') return escapedAsCodePoint(c) if (index === 0 && string.length === 1) return escaped(c) if (c >= '\u0080' || c === '-' || c === '_' || inRange(c, '0', '9') || inRange(c, 'A', 'Z') || inRange(c, 'a', 'z')) return c; return escaped(c) }) function inRange(c, low, high) { return c >= low && c <= high } function escaped(c) { return '\\' + c } function escapedAsCodePoint(c) { return '\\' + c.charCodeAt(0).toString(16) + ' ' } } and then I'd check to see if I really needed to complicate the tests and/or pull out special cases to get a more speed. By the way, the replace(/./g) is not an important part of what I'm saying. I used that method to minimise the differences between our approaches. When if comes the language features, I would probably write this: function escape4 (value) { const string = String(value) return string.replace(/./g, (c, index) => { return c === '\0' ? '\ufffd' : inRange(c, '\1', '\1f') ? escapedAsCodePoint(c) : index === 0 && inRange(c, '0', '9') ? escapedAsCodePoint(c) : index === 1 && inRange(c, '0', '9') && string[0] === '-' ? escapedAsCodePoint(c) : index === 0 && string.length === 1 ? escaped(c) : c >= '\u0080' || c === '-' || c === '_' || inRange(c, '0', '9') || inRange(c, 'A', 'Z') || inRange(c, 'a', 'z') ? c : escaped(c) }) function inRange(c, low, high) { return c >= low && c <= high } function escaped(c) { return '\\' + c } function escapedAsCodePoint(c) { return '\\' + c.charCodeAt(0).toString(16) + ' ' } } but this is not really my point. Readability goes much deeper than the surface constructs. It's about, as I think you said, how easy the code is to verify, and that often means how closely it matches the specification. If this was hard-line production code, I think I'd include the seven sentences from the specification as a comment. That would go a long way to explaining why the code is written as it is. (I'm aware that posting code that has not had much testing is offering a hostage to fortune. But, if I've made a mistake, I hope my point will be made by how easily you spot it!) -- Ben.