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

IndicatorResult needs refactoring #13

Open
amv-dev opened this issue Jan 24, 2021 · 1 comment
Open

IndicatorResult needs refactoring #13

amv-dev opened this issue Jan 24, 2021 · 1 comment
Labels
enhancement New feature or request

Comments

@amv-dev
Copy link
Owner

amv-dev commented Jan 24, 2021

For now every indicator must implements both values and signals calculation logic. That might be a problem if you want to use values to implement your own signals logic because of performance degradation.

So the current proposal is to make an IndicatorResult as a trait which will only be responsible for representing values. The idea behind is that every indicator should have it's own IndicatorResult type. This will help keep indicators values more verbose.
A single struct, that implements IndicatorResult, must contain all the values, that indicator returns. As a bonus there might be different types of values: not only f64 like now.

Then for signals create another trait Signal and make two internal signal types: BinarySignal for signals like {-1; 0; +1} and FloatSignal for signals in range [-1.0; 1.0]. Every single signal must have it's own struct representation.

This approach has next pros:

  • Every indicator may produce different type of values: not only f64s like now;
  • Every IndicatorResult may have more verbose documentation and explanation;
  • You may choose which signals you want to calculate and which not waste performance on;
  • More clear differentiation on signal types;
  • Custom signal types might be created for your personal indicator.

Cons:
More code must be written to get a signal from indicator: you need to create IndicatorConfig, then initialize IndicatorInstance (like now), then calculate IndicatorResult, then initialize appropriate Signal, then throw IndicatorResult into Signal and finally get your signal.

@amv-dev amv-dev added enhancement New feature or request indicator Something with indicator implementation labels Jan 24, 2021
@amv-dev amv-dev changed the title IndicatorResult need refactoring IndicatorResult needs refactoring Jan 25, 2021
@amv-dev amv-dev removed the indicator Something with indicator implementation label Jun 8, 2021
@raimundomartins
Copy link

I agree with this. Personally, I don't care about Keltner Channel's signals, but want to use its values.

I'm also not a fan of the (arbitrary) 4 signals limit nor the enum Action, which feels it can be completely replaced by a f32 and would be improved. Custom structs for indicators are a definite improvement!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants