Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use jobleonard improvements #35

Open
Artoria2e5 opened this issue Apr 4, 2020 · 1 comment
Open

Use jobleonard improvements #35

Artoria2e5 opened this issue Apr 4, 2020 · 1 comment

Comments

@Artoria2e5
Copy link
Contributor

Artoria2e5 commented Apr 4, 2020

Over at https://observablehq.com/@jobleonard/mario-klingemans-stackblur there is a modernized shift table. There are a few things useful to this repo:

  • Correctness: the shift operator is corrected from >> to >>> so integers stay okay until the intended 255 radius. And lots of | 0 to make sure.
  • Simplicity:
    • The shift table is removed in favor of a fixed 24-shift so values become more readable
    • Since stacks only work one way, an array is used instead
    • Fewer stray variables for cleaner code
  • Modern JS
@brettz9
Copy link
Collaborator

brettz9 commented Apr 4, 2020

In what way is the JS more modern? The source here is pretty modern.

Before accepting any non-obvious changes, I personally think we need some mocha + nyc testing, using some canvas or image testing library, to ensure current behavior is working as intended.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants