[WIP] fix: decimal step cause divide not equal by paranoidjk · Pull Request #228 · react-component/slider

paranoidjk · 2017-02-27T03:16:14Z

close #227

coveralls · 2017-02-27T03:19:26Z

Coverage decreased (-0.6%) to 49.793% when pulling 7395d99 on fix/decimal-step into 4e26da5 on master.

coveralls · 2017-02-27T03:24:15Z

Coverage decreased (-1.04%) to 49.383% when pulling a132e78 on fix/decimal-step into 4e26da5 on master.

coveralls · 2017-02-27T04:25:29Z

Coverage increased (+5.5%) to 55.967% when pulling d57068d on fix/decimal-step into 4e26da5 on master.

paranoidjk · 2017-03-01T04:28:47Z

ping @benjycui

benjycui · 2017-03-01T07:45:12Z

src/Range.jsx

    const { marks, step, min, max } = this.props;
    const cache = this._getPointsCache;
    if (!cache || cache.marks !== marks || cache.step !== step) {
+      const decimal = step.toString().split('.')[1] && step.toString().split('.')[1].length;


Use getPrecision ？

benjycui · 2017-03-01T07:47:15Z

src/Range.jsx

      if (step !== null) {
-        for (let point = min; point <= max; point += step) {
+        let point = min;
+        while (point <= max) {


This loop will cost a lot.

e.g. min = 0 && max = 99999

the performance problem only occur when users set step to be a very small decimals, I think it's not our worries ?

maybe use something like binary search + Array.map to caculate the point more quickly?

~~caculate the point more quickly~~ ——> avoid Maximum call stack

Another idea, what if when step is very small compare to max - min, we don't calculate the whole points array at first, instead just get the formula，something like： value = f(index), then the cost just happen when every time getter

paranoidjk · 2017-03-01T09:31:18Z

close. going to open a new pr to fix the decimal step problem.

paranoidjk assigned benjycui Feb 27, 2017

paranoidjk force-pushed the fix/decimal-step branch from 7395d99 to a132e78 Compare February 27, 2017 03:20

paranoidjk force-pushed the fix/decimal-step branch from a132e78 to d57068d Compare February 27, 2017 04:20

benjycui reviewed Mar 1, 2017

View reviewed changes

paranoidjk force-pushed the fix/decimal-step branch from d57068d to 3ef1197 Compare March 1, 2017 08:01

paranoidjk changed the title ~~fix: decimal step cause divide not equal~~ [WIP] fix: decimal step cause divide not equal Mar 1, 2017

fix: decimal step cause divide not equal. close #227

778c729

paranoidjk force-pushed the fix/decimal-step branch from 3ef1197 to 778c729 Compare March 1, 2017 08:50

paranoidjk closed this Mar 1, 2017

paranoidjk deleted the fix/decimal-step branch March 1, 2017 09:31

Uh oh!

Conversation

paranoidjk commented Feb 27, 2017

Uh oh!

coveralls commented Feb 27, 2017

Uh oh!

coveralls commented Feb 27, 2017

Uh oh!

coveralls commented Feb 27, 2017

Uh oh!

paranoidjk commented Mar 1, 2017

Uh oh!

benjycui Mar 1, 2017

Choose a reason for hiding this comment

Uh oh!

paranoidjk Mar 1, 2017

Choose a reason for hiding this comment

Uh oh!

benjycui Mar 1, 2017

Choose a reason for hiding this comment

Uh oh!

benjycui Mar 1, 2017

Choose a reason for hiding this comment

Uh oh!

paranoidjk Mar 1, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

paranoidjk Mar 1, 2017

Choose a reason for hiding this comment

Uh oh!

paranoidjk Mar 1, 2017

Choose a reason for hiding this comment

Uh oh!

paranoidjk Mar 1, 2017

Choose a reason for hiding this comment

Uh oh!

paranoidjk commented Mar 1, 2017

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

paranoidjk Mar 1, 2017 •

edited

Loading