Palindrom / JSONPatcherProxy

ES6 proxy powered JSON Object observer that emits JSON patches when changes occur to your object tree.
94 stars 14 forks source link

Refactor for better code readability #36

Closed warpech closed 5 years ago

warpech commented 5 years ago

I have made several improvements that were discussed with @tomalec over Slack. These changes are intended to increase code readability, which is needed before we can do further work.

warpech commented 5 years ago

Compatibility

I have searched Palindrom and Starcounter GitHub orgs, and could not find any usage of the APIs that I've changed.

Proposed more changes

We could change all properties to begin with _, because they are intended for private use. Private methods do begin with _.

Benchmarks

Before this PR

jsonpatcherproxy generate operation
 Ops/sec: 136838 ±2.76% Ran 8652 times in 0.063 seconds.
jsonpatch generate operation
 Ops/sec: 109856 ±6.94% Ran 6494 times in 0.059 seconds.
jsonpatcherproxy mutation - huge object
 Ops/sec: 1283 ±0.26% Ran 67 times in 0.052 seconds.
jsonpatch mutation - huge object
 Ops/sec: 887 ±1.73% Ran 46 times in 0.052 seconds.
jsonpatcherproxy generate operation - huge object
 Ops/sec: 1084 ±0.99% Ran 58 times in 0.054 seconds.
jsonpatch generate operation - huge object
 Ops/sec: 860 ±1.65% Ran 47 times in 0.055 seconds.
PROXIFY big object
 Ops/sec: 2536 ±0.40% Ran 131 times in 0.052 seconds.
DIRTY-OBSERVE big object
 Ops/sec: 1746 ±1.18% Ran 94 times in 0.054 seconds.

After this PR

jsonpatcherproxy generate operation
 Ops/sec: 145558 ±2.37% Ran 9371 times in 0.064 seconds.
jsonpatch generate operation
 Ops/sec: 107443 ±5.93% Ran 6450 times in 0.060 seconds.
jsonpatcherproxy mutation - huge object
 Ops/sec: 923 ±1.95% Ran 52 times in 0.056 seconds.
jsonpatch mutation - huge object
 Ops/sec: 878 ±1.48% Ran 46 times in 0.052 seconds.
jsonpatcherproxy generate operation - huge object
 Ops/sec: 826 ±1.97% Ran 48 times in 0.058 seconds.
jsonpatch generate operation - huge object
 Ops/sec: 874 ±1.65% Ran 48 times in 0.055 seconds.
PROXIFY big object
 Ops/sec: 1845 ±2.16% Ran 103 times in 0.056 seconds.
DIRTY-OBSERVE big object
 Ops/sec: 1738 ±1.10% Ran 94 times in 0.054 seconds.
warpech commented 5 years ago

To verify that the TS type definitions are correct, I created a file locally src/test.ts with the following content and checked that it compiles:

import JSONPatcherProxy from '../';

var obj = {};
var instance = new JSONPatcherProxy(obj);
instance.generate();
instance.observe(true);

Basically this is the same as the snippet presented in README.md.

warpech commented 5 years ago

Thank you for the reviews! Merging after the tests pass, as all the comments have been addressed

alshakero commented 5 years ago

It seems .vscode/settings.json was added unintentionally.

warpech commented 5 years ago

I actually did that intentionally, thinking that a common project config might be useful. Do you think it is better to remove it? @tomalec?

tomalec commented 5 years ago

I'm not sure. Usually, I'm not a fan of bloating project repo with IDE configs, but If think it's better to keep it, I'm good.