I review code

Joined December 2020
24 Photos and videos
7/10 * Suggest refactoring: if os.name == "nt": cmd = "start" elif os.uname()[0] == "Linux": cmd = "deepin-image-viewer" if "deepin" in os.uname()[2] else "eog" else: cmd = "open" os.system(cmd " " image_file) @huangHyw github.com/huanghyw/jd_secki…

7/10 * `tickMin`, `tickMax`, and `tickStep` should be local variables in the constructor. * Suggest using ternary to initialize `exp`: ``` double exp = (range == 0.0) ? 0 : Math.floor(Math.log10(range / (tickCount - 1))); ``` @henningpohl github.com/arduino/Arduino/b…
9/10 * To avoid later memory reallocation, line 44 should have `make([]rune, 0, (width 1)*height)` to account for the '\n' characters. * Reversing direction could be written more simply as `vx = -vx` and `vy = -vy` @inancgumus github.com/inancgumus/learng…
1
7/10 * Suggest inverting test in `if` on line 148, and breaking out of loop to reduce indentation depth. * Suggest assigning variable to `len(iterable)` at top of function @fvsmassa github.com/facebookresearch/…
8/10 * For clarity, suggest changing line 23 to ``` if (!control) { return; } const controlType = control.type.toLowerCase(); ``` github.com/storybookjs/story…
8/10 * `elif`s can be replaced by `if`s, and `else`s can be removed github.com/google-research/b…
9/10 * Could precompute `2 * standardDeviation * standardDeviation` outside the for loop and assign it to a variable to avoid recomputing each time around the loop github.com/mapsme/omim/blob/…
9/10 * stray `var` on line 174 that could be a `const` * perhaps use simple exponential backoff when polling for webpack completion, rather than always waiting one second @sugarpirate_ github.com/reactjs/server-co…
9/10 * The `silent || ...` idiom seems unnecessary. A simple `if (!silent) ...` or `if (verbose) ...` would be easier to read and not much more typing. @aarondjents github.com/facebook/jest/blo…
6/10 * Unused variable `channels` * Overly complicated filter callback with side effect. Could do something simpler such as ``` playlist.items = playlist.items.filter((a, index) => index === playlist.items.findIndex(b => b.url === a.url)); ``` github.com/iptv-org/iptv/blo…
8/10 * Non-standard spacing to line things up vertically makes it harder to visually parse the code * Lines too long @mikeroberts3000 github.com/apple/ml-hypersim…
1
7/10 * Redundant `else` after `return` * Body of function could be simplified to something like: ``` if v.Width == 0 && v.Height == 0 { return "" } return str(x.Width) ":" str(x.Height) ``` where `str` returns either "-2" or the Itoa. @gabek github.com/owncast/owncast/b…
4/10 * Logic of Boolean expression on line 62 is wrong (it always evaluates to true) * Use of same `cached` variable for two different purposes (the body content and a Boolean). Could have avoided the second use by just `return`ing at line 71. github.com/r-spacex/SpaceX-A…
8/10 * Redundant `else` after `return` * Using an immediately invoked function expression seems unnecessarily complex. Suggest putting this code into a separate named function to be called from here, or just inline and restructure the code. @nwtgck github.com/nwtgck/piping-ser…
8/10 * Variable declarations could be combined in `if` test line: ``` if newPath, err := r.resolvePath(value); if err != nil { newPath, err = resolvePathWithBaseFolder(… … } else { value = newPath } ``` github.com/projectdiscovery/…
7/10 * Single-line `if` body * `time` could be `const` * Reuse of variables for different purposes * Odd use of regex to iterate over a string * Lines 28 and 29 could be simplified to something like `array[0] = 1 (array[0] - 1) % 12;` @GitSquared github.com/GitSquared/edex-u…
1
1
8/10 * Extremely long (233 character) line of code * Non-syntactic macro * Some one-line `if`s have body on same line @promag github.com/bitcoin/bitcoin/b…
6/10 * Non-syntactic macros * Public fields * Missing `const`s * Capitalization of field names @wfigat github.com/FlaxEngine/FlaxEn…
8/10 * Function is too long and nesting of `for` and `if`/`else` is too deep. Suggest refactoring to split it up into smaller functions. github.com/sherlock-project/…
6/10 * `var' declarations should be moved to top of functions to match their scope (or use `let`/`const` if supported) * Better to use === for comparisons @r0ysue github.com/r0ysue/r0capture/…