7 ms·
For the purposes of discussion, it'd probably be helpful to describe the issues you would identify during review
by cauthon 3y ago
For the purposes of discussion, it'd probably be helpful to describe the issues you would identify during review
- laserlight 3y agoI’ve just found one. Shortening “input” to “inp” [0] is a big no. [0] https://github.com/dabeaz/blog/blob/main/2023/three-problems.md https://github.com/dabeaz/blog/blob/main/2023/three-problems...
- benji-york 3y agoThe symbol "input" is a Python built-in.
- laserlight 3y agoGood catch. I would prefer shadowing an unused built-in over using “inp”. Alternatively, “input_” should do fine.
- ilc 3y agoDon't shadow. Just don't. We've got enough variable names in the universe. :)
- laserlight 3y agoStrong agree. To clarify, I wouldn't shadow. I wouldn't use “inp” either.
- ali_m 3y agoOne fairly common convention is to suffix with `_` to avoid shadowing, e.g. `input_`
- EddieJLSH 3y agoPlease don't shadow ever :(
- mrelectric 3y agoMan..
- dacryn 3y agoshadowing is what instantly would have me reject a peer review. there is literally no excuse. inp is just standard in python, along with uin (user input), or sometimes also raw or iraw. You cannot win this battle. There is nothing wrong with 'inp'
- BbzzbB 3y agoGuess it's pretty decent code when the point of contention is about a single variable name being abbreviated.
- laserlight 3y agoThis was the first and most obvious problem I found upon looking for a few seconds. I don't know if this is the only issue with the code.
- edgyquant 3y agoThat’s seriously the glaring issue you’ve found? It’s a tutorial ffs. Have you actually done code reviews professionally?
- laserlight 3y agoI didn't say it was a glaring issue. But, unnecessary cognitive burden on the reader is unacceptable. > Have you actually done code reviews professionally? Yes, I did.
- knighthack 3y agoThat's the silliest thing for you to bikeshed about. In a serious code review, that isn't even a starter of an issue, if you have context surrounding that variable. Furthermore `input` is an actual Python function, and shortening an example for learning purposes is not the same as asking other people do the same in production code.
- twelve40 3y agohaha you should see the code review comments at my <current job>. Random people spend hours bickering over variable names, while completely overlooking real issues in the code and rubberstamping their friends. It's like a friggin cult.