amv-dev / yata

Yet Another Technical Analysis library [for Rust]
Apache License 2.0
334 stars 52 forks source link

`IndicatorResult` needs refactoring #13

Open amv-dev opened 3 years ago

amv-dev commented 3 years ago

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:

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.

raimundomartins commented 3 years ago

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!