Refactor/core #59
Refactor/core #59
Conversation
Code has been refactored and commented to be easier to follow.
All datas and functions related to error correction have been moved to a dedicated file. EC blocks and EC codewords values, have been splitted into two tables for better handling. Methods inside Polynomial class have been rewrited as static functions since are just used like maths utility methods. `Polynomial.mod` has been refactored to avoid recursive calls. EC generation is now handled by `ReedSolomonEncoder` class. Code has been optimized to create a generator polynomial just once instead of re-calculate it at each iteration. Arrays and `bops` lib have been replaced with Buffer.
Browserify and Webpack shims for node Buffer include a lot of methods that will be likely never used by this lib. This commit add a stripped version of the original Buffer shim, with only the methods strictly needed. Bundle size is reduced by ~50%, resulting in a footprint of only ~7Kb (minified and gzipped).
|
wow! ill take a look now |
|
one issue is that prepublish runs on npm install also. this means that folks have to run the tests just to install it as a dep also we build the browser version before we publish so that folks dont have to when they install and they dont need the dev deps like browserify im not sure if i left these wires crossed before you got to it (probably) but lets figure out how to solve it before this release |
|
It makes sense. I'll remove the test task from prepublish. |
|
I've just noticed there was a bug in encode method that would occur using numeric or alphanumeric mode. Probably also in some rare cases in byte mode. Fixed now. (num and alphanum modes are not implemented yet, I'm working on them in another branch, I hope to push it soon) |
|
this is amazing work. i just wanted to say thanks! and im glad you are digging in |
|
it's a pleasure 😄 |
|
if you think this is ready to go i approve. please go ahead and merge. |
|
@soldair I've added a few changes. If it's still ok for you I'm ready to merge this. |
|
hmm. if anyone is using the |
|
mm ok, you've right, I hadn't thought of that. I probably have to revert it to the old name for now. I've intentionally kept all the API untouched for this first part of the refactor. The only thing that I've removed is one of the global var exported for the browser. A rough roadmap for the next changes could be:
|
|
awesome 👍 |
|
🐑 |
|
👏 🎊 |
This is the first part of the refactor proposed in #56.
It is focused on lib structure and reorganization of core files.
The initial qrcode.js file has been splitted in many separated modules to make code easier to follow and update.
Some parts have been reimplemented and where possible some optimisations have been added to make the qrcode generation faster.
Also, I tried to comment and document as much as possible the code to explain what it does.
For each core module has been added an initial test file which cover the implementation.
More tests should be added in future to cover possible edge cases.
Example files have been moved to a dedicated folder and all files not needed have been excluded from publishing to have a cleaner package.