bennycode / trading-signals

Technical indicators to run technical analysis with JavaScript & TypeScript. 📈
https://bennycode.com/trading-signals
MIT License
621 stars 90 forks source link

Bug of update if the replace parameter is true when use FasterEMA #681

Closed nicewookyes closed 6 months ago

nicewookyes commented 6 months ago

The function setResult In indicator.ts will export wrong value while the 'replace' parameter is true, beacuse 'this.previousResult' always replaced by 'this.result'. If 'replace' is true in continuous calls to 'update', you will get wrong result after second call

update(_price: BigSource, replace: boolean = false): Big {
    if (!replace) {
      this.pricesCounter++;
    } else if (replace && this.pricesCounter === 0) {
      this.pricesCounter++;
    }
    const price = new Big(_price);

    if (replace && this.previousResult) {
      return this.setResult(
        price.times(this.weightFactor).add(this.previousResult.times(1 - this.weightFactor)),
        replace
      );
    }
    return this.setResult(
      price.times(this.weightFactor).add((this.result ?? price).times(1 - this.weightFactor)),
      replace
    );
  }
  protected setResult(value: number, replace: boolean = false): number {
    if (replace && this.previousHighest !== undefined) {
      this.highest = value > this.previousHighest ? value : this.previousHighest;
    } else if (this.highest === undefined || value > this.highest) {
      this.previousHighest = this.highest;
      this.highest = value;
    }

    if (replace && this.previousLowest) {
      this.lowest = value < this.previousLowest ? value : this.previousLowest;
    } else if (this.lowest === undefined || value < this.lowest) {
      this.previousLowest = this.lowest;
      this.lowest = value;
    }

    this.previousResult = this.result;
    return (this.result = value);
  }

Got correct result after change to this

    if(!replace){
      this.previousResult = this.result;
    }
bennycode commented 6 months ago

Thanks for the hint and the fix. Do you have a little test case in place which I can use? I think I am fixing the issue with #679

bennycode commented 6 months ago

Hi @nicewookyes, I found the issue and fixed it: https://github.com/bennycode/trading-signals/pull/679

I also added test cases for the EMA replace behaviour: https://github.com/bennycode/trading-signals/pull/683

Please install trading-signals@5.0.3 and let me know if it works. 😀