Re:How paywalled sites permit access to visitors from social...
by elaineo on Wednesday July 19, 2017 @10:03PM
(#14809551)
Awesome code review, even more valuable than the plugin itself. Thanks :)
Re:How paywalled sites permit access to visitors from social...
by mattmanser on Tuesday July 18, 2017 @04:59AM
(#14795185)
You can actually just do this:
for (var i = 0, l = details.responseHeaders.length; i < l; ++i) {
Re:How paywalled sites permit access to visitors from social...
by chrismorgan on Tuesday July 18, 2017 @07:35AM
(#14795732)
I would have done that, but because of the splicing you don’t actually want ++i every time. That’s why I rewrote it to a while loop.
Even then you could do it as a for loop with an empty increment clause, of course. Or even use a one-liner for loop with no body:
function blockCookies(details) {
for (var headers = details.responseHeaders, i = headers.length - 1; i > -1; headers[i].name === "Set-Cookie" ? headers.splice(i, 1) : i--);
return {responseHeaders: headers};
}
But that’s crazy talk. Leave optimisations that to the minifier if it feels so disposed.
This pattern:
should be replaced by this: This pattern: should be replaced by this: (Note the semantics are slightly different—`.some` will break early, so it’s more efficient and equivalent provided there are no side-effects in the map function.)Taking both of these, the following:
can be rewritten much more simply as: You could even do it thus if you desired: … but that’s probably harder to read. I will mention arrow functions, however, which are pretty: This part: `.filter` only cares about truthiness in its return value—as this code does it, undefined is false and an object is true. But you could simplify it: Also in the original code’s usage of map, it’s not actually changing the values, only things inside them, so using `map` is wasteful (as it entails allocating a new array). You could just use `forEach`: A remark on fine-tuning performance: when you access properties inside an object multiple times, it’s optimal to store it as a local variable to save having to look it up multiple times. (This is especially the case if the property is expensive to access.) Take the `blockCookies` method: This is accessing `details.responseHeaders` many times when it only needs to access it once. It is also accessing its `length` member once per iteration, rather than caching the length. Normally for that I’d say “store the length once up front,” but in this case the code is changing the array length in the loop, so that’d actually break things. On that note, the code as published is actually missing some cookies, because it removes an item from the array and then skips past the new element at that index. To fix that, you need to move the `++i` into the loop so it can be skipped if you do splice the array. Also in order to not need to access the length property many times you could iterate in reverse instead of forwards. I might write the whole function like this: