ta-rs
ta-rs copied to clipboard
Why is a complete DataItem needed?
Just for example, OnBalanceVolume uses only close and volume. Yet I need to build a complete DataItem using the builder() method.
And this runs the validation if clause in DataItemBuiler::build() – no matter what.
It would be nice to be able to use the init struct pattern on DataItem to forego using a DataItemBuilder and allowing to skip the validation code.
In my case the data I feed into ta comes from tickers. So validation is not needed/correctness is guaranteed.
All that would be needed is adding a Default trait and a is_ok() method to DataItem and remove the validation code from DataItemBuiler::build(). That method would always just return a DataItem. Otherwise the method should be named build_and_validate() to express the hidden runtime penalty it entails.
As such TaError::DataItemIncomplete would not be needed any more since we acknowledge that some indicators work just fine with 'incomplete' DataItems and Data::Item::is_ok() could just return false if the check fails. Analogous what is now TaError::DataItemInvalid.
This suggestion would break the API/existing code of course.
The other option is to just add the Default trait and is_ok() methods to DataItem. This way new code using ta could use the aforementioned init struct pattern. A call to is_ok() can be wrapped in compile time flags to make sure it doesn't end up in release builds.
Does that make sense?
Created a PR for your consideration.
@virtualritz Hi!
Thanks for your comment, the suggestion you propose sounds reasonable, enforcing validation without giving any other options, indeed not the best idea. On other hand.. I did not expect people to use DataItem extensively, they should rather come up with own implementations of "DateItem"s depending on their source of data (.e.g it's often that Volume is missing).
Thanks for the PR. I will take a close look at it next week, when I'll have time. It may be, that I'd go for multiple variations of data items, depending on data that are supplied.
Yet I need to build a complete DataItem using the builder() method.
You don't have to. DataItem here is mostly for illustrative purpose and a little bit of convenience.
The indicators are designed not to rely on DataTime, but rather on the abstractions (Close, Low, High, etc traits).
I just added an example, please see: https://github.com/greyblake/ta-rs/blob/master/examples/custom_data_item.rs
It would be nice to be able to use the init struct pattern on DataItem
This pattern for this use case creates a lot of surfaces for potential errors. For example: you may initialize DataTime with only high, and use TrueRange indicator, which will pick 0 for low and close. This will of course will lead to incorrect output without any errors.