4 ms·
A comment on code organisation. https://github.com/victorqribeiro/TinyApp/blob/db92225cfd0aed935b7dd6f70e36c0a852a5e462/js/Input.js https://github.com/victorqr
by chrismorgan 2y ago
A comment on code organisation.
https://github.com/victorqribeiro/TinyApp/blob/db92225cfd0aed935b7dd6f70e36c0a852a5e462/js/Input.js https://github.com/victorqribeiro/TinyApp/blob/db92225cfd0ae...:
export default function Input(state) {
const inp = input({ // ①
type: 'number', value: state.count, onchange
})
function update() { // ②
inp.value = state.count
}
function onchange() { // ③
state.count = parseInt(this.value)
}
state.addUpdate('count', update) // ④
return inp
}
(Labelling comments mine.)
This ordering isn’t great: ① refers to ③, and ④ refers to ②. It’d be better to reorder them, at least to swap ② and ③, and remove the blank line separating ② and ④:
export default function Input(state) {
const inp = input({ // ①
type: 'number', value: state.count, onchange
})
function onchange() { // ③
state.count = parseInt(this.value)
}
function update() { // ②
inp.value = state.count
}
state.addUpdate('count', update) // ④
return inp
}
But you really can do much better:
export default function Input(state) {
const inp = input({
type: 'number',
value: state.count,
onchange() {
state.count = parseInt(this.value)
}
})
state.addUpdate('count', function() {
inp.value = state.count
})
return inp
}
At least at this scale, that’s much easier to follow.
Inlining can improve other places too (though at larger scales it becomes more debatable):
export default function Button(state) {
return button({
onclick() {
state.count += 1
},
style: {
color: 'black',
border: 'solid 1px black'
},
}, '+ 1')
}
- deleted 2y ago[deleted]
- contrarianmop 2y ago[dead]
- atum47 2y ago> (though at larger scales it becomes more debatable) My thought exactly. In a small component as the Button I agree with inline everything (almost did it myself cause I knew people would read this code) but I have decided to keep it as is for readability. In a more complex component as the Canvas I think you'd agree with me that the long winded version is easier to grasp what's going on.
- chrismorgan 2y agoIn Canvas, I’d still prefer to inline everything, but I recognise that you’re getting to the scale where it’s more debatable—that it’s not just unquestionably superior, as it is in the smaller cases like this. (You “decided to keep it as is for readability”? I’m confused; I find the way you wrote it vastly less readable, it’s not even close.) When you don’t inline the methods, really all that you get out of it is a list of properties, without the method bodies getting in the way. The question then is, is that useful? If you’re in a simple text editor, then sometimes definitely yes. But it’s duplication, it’s more scope for mistakes, and all up there’s a reason why things like C headers have been falling out of favour as an authoring style. More advanced editors these days can present file outlines, which will work pretty well with the function style, and would work even better with class methods, but may not work so well with the inlined object-shorthand methods (I don’t know, I’m not in the habit of using such outliners—it’s possible for them to take these methods, but it wouldn’t surprise me to find them omitting them altogether). It’s a similar deal to whether you write `export function a… export function b…` or `function a… function b… export {a, b}`. Some prefer one, some the other. Sometimes there are compelling reasons to use one or the other, independent of preference. I get the feeling immediate export is more popular.
- gknoy 2y agoThis is somewhat off-topic, but I am blown away by your use of uncommon characters like "①". They stand out so much that they (for me at least) make referencing parts of the code snippet so much easier to follow. I see these _nearly never_, so rarely that I forgot they were available to use. I didn't realize HN supports them, or that I could probably use them in review comments on Github. Thank you for the inspiration! (now to figure out how to actually type them...)
- chrismorgan 2y agoI typed them with my Compose key, which I bind to the key to the right of Space, typically RAlt: Compose ( 1 ) ⇒ ① (this is part of the default mappings in /usr/share/X11/locale/en_US.UTF-8/Compose). I recommend investing in a good Compose key setup. When I was on Windows, I used WinCompose. On Linux, you just need to bind the key and it’ll work. macOS, no idea. HN does block emoji, for better or for worse, but almost everything else is allowed. The thing that frustrates me most about HN’s Unicode restrictions are its normalisation of diverse spaces to U+0020 SPACE; if I typed THIN SPACE or NARROW NO-BREAK SPACE or NO BREAK SPACE or whatever (and I do) that’s what I wanted; and mangling U+200B ZERO WIDTH SPACE and U+200C ZERO WIDTH NON-JOINER is just bad, and has prevented me from accurately representing Telugu text before—Indic scripts use the latter.
- lrpe 2y agoI'm more annoyed by them. They're so small on my work monitor that it's nearly impossible to tell them apart. They just look like smudges. I don't get the need to use novel symbols like this when standard numerals would suffice.
- chrismorgan 2y agoI’d be interested to see a screenshot. Something’s quite wrong if they look like smudges. I’d be interested to see what font’s being used, too, which you can find in Firefox’s dev tools, Inspector tab, Fonts pane, Fonts Used section.