3 ms·
Maybe I'm not quite understanding the first function, but you could probably write it like this: (accum, borders) => ({ ...accum, [borders.datatype]:
by scottmf 9y ago
Maybe I'm not quite understanding the first function, but you could probably write it like this:
(accum, borders) => ({
...accum,
[borders.datatype]: (accum[borders.datatype] || []).concat([borders])
}, {})
although it's a little codegolfy and I'd personally write it more like this:
(accum, borders) => {
const datatype = accum[borders.datatype] || [];
datatype.push(borders);
return {
...accum,
[borders.datatype]: datatype,
};
}, {})
- KirinDave 9y agoPush modifies the prior data structure. I could use it here but I do my best to avoid using push unless I'm very deliberately mutating an array iteratively. In this case it would work, but I don't like afforidng myself mutability as a short cut. Nice point on the first part though.
- bpicolo 9y agoLocal-scoped mutability is not typically an issue. It's mutability at higher levels that cause things to be tough to track down. Seems like that would be generally overkill to worry about, especially in languages where immutability isn't native
- KirinDave 9y ago> Local-scoped mutability is not typically an issue. But our refactoring tools really don't reveal the extend of variables, so while I may be confident in this outcome in the case where I write it, I leave a bit of a caveat that's difficult to document for subsequent developers. Same with my use of const that was noted above, it's more a message to future developers than a useful assertion (const does very, very little in js). If we were using Purescript or Haskell where I could assert this caveat with types, I'd do that instead. Even in Clojure, where it's actually kinda painful to leak the mutable contexts, I might do it. Too easy to lose control of stuff during a refactor in Typescript, so I don't.
- kevmo314 9y agoYou could do (accum, borders) => ({ ...accum, [borders.datatype]: [...(accum[borders.datatype] || []), borders] }, {}) although it's essentially concat().
- KirinDave 9y agoI actually like that quite a bit more. I should have thought of it. Thanks!
- scottmf 9y agoBy the way, you don't need to use let in your original code. const x = {}; x.a = 123; You're not changing x, you're changing a property of the object it points to.
- KirinDave 9y agoThis is more a declaration of intent. I am aware that const doesn't stop property modifications, and I think it's ridiculous. I try to use "const" to mean "I never intend to change this." And I keep lobbying people to give us a better const keyword. But thank you, I appreciate the feedback.