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

CachedIndicator should be a wrapper indicator not in inheritance structure #608

Open
GxtPerfectoM opened this issue Jun 22, 2020 · 10 comments
Labels
enhancement Issue describing or discussing an enhancement for this library

Comments

@GxtPerfectoM
Copy link

Currently it looks like the CachedIndicator is causing problems in multi-thread mode.
In general you should not use the CachedIndicator in inheritance structure but instead offer it as a wrapper to other indicators (Delegator) and this will give the way to avoid using it if needed.

@GxtPerfectoM GxtPerfectoM added the enhancement Issue describing or discussing an enhancement for this library label Jun 22, 2020
@nimo23
Copy link
Contributor

nimo23 commented Jan 20, 2021

@GxtPerfectoM would be nice if you can make a PR with your suggestion.

@johnthetireless
Copy link
Contributor

johnthetireless commented Apr 20, 2021

Here is a naive "caching decorator" around Indicator.
You can adapt for and change the caching policy as you see fit.

public interface NumericIndicator extends Indicator {
}

public class CachedNumericIndicator implements NumericIndicator {

private final List<Num> cache = new ArrayList<>();

public CachedNumericIndicator(Indicator<Num> delegate) {
	this.delegate = delegate;
}

/*
 * Simplistic implementation will probably suffice for most use cases.
 * Replace or override if desired
 */
@Override
public Num getValue(int index) {
	if ( index < cache.size() ) {
		return cache.get(index);
	}
	else {
		Num result = delegate.getValue(index);  
		cache.add(result);
		return result;
	}
}

@Override
public Num numOf(Number number) {
	return delegate.numOf(number);
}

@Override
public BarSeries getBarSeries() {
	return delegate.getBarSeries();
}

}

@johnthetireless
Copy link
Contributor

Generic types didn't copy/paste correctly.

@johnthetireless
Copy link
Contributor

Caching

There are 6-7 "helper" indicators like MedianPrice whose calculation is based solely of the price/volume data of a single bar. Caching the result of Open(i) + Close(i) / 2 seems questionable.

A larger set of higher-level indicators do the same, but they do arithmetic on values obtained from "child" indicators.
PlusDI(n) = MMA)n) / ATR(n) * 100, for example. I would be inclined to let this calculate each time without being cached.

I would really like to see caching removed from the Difference, Sum and other "arithmetic" indicators. They don't seem to be used much, but series arithmetic is a powerful concept. A(i) = B(i) + C(i) is very easy to code as an operation of 2 series.

@nimo23
Copy link
Contributor

nimo23 commented May 4, 2021

See #522 (comment).

See #103 (comment)

@johnthetireless
Copy link
Contributor

there are so many easy fixes that you or anyone could make to this library to clean up the array allocations its boggles my mind... in an afternoon you could cut out 25% of the wasted space that causes problems you don't have because you ado the right things.... yet you will comment on this, and draw false dependencies on it, waiting for someone else to come along and submit a magic fix to "turn caching off" which is not possible... do something... i am amazed at the attitude... someone points out a flaw and you tell them to submit a PR and it gets ignored or worse, attacked... i suspect this issue will sit here while nobody does anything because they can't... the process is impossible... you are stuck with some very bad code in some places because no one will change it and no one can..

i wish you luck... i am done

@nimo23
Copy link
Contributor

nimo23 commented Jan 15, 2022

there are so many easy fixes* that you or anyone could make to this library to clean up the array allocations its boggles my mind..you are stuck with some very bad code in some places

@johnthetireless I think you didn't really understand the full point of caching. You can re-calculate if a value is not cached or cannot be found in the cache - I don't talk about the caching of a value which is needed to calculate the other value in another place (this is the kind of caching you find on a typcial calculater with the name "MR = Memory Recall"). But nevertheless, please tell us: what exact classes or methods in this project you are talking about? What kind of (the "so many easy") fixes for what code do you mean exactly?

you are stuck with some very bad code in some places because no one will change it and no one can..

In general, it would help if you could get the facts straight in a constructive way so we know where (in the code) improvements can be made instead of blaming us in such a condescending way.

@johnthetireless
Copy link
Contributor

johnthetireless commented Jan 15, 2022

Please don't tell me I don't understand.. well anything... the title of your last issue "disable caching in BarSeriesManager#run" tells anyone you have no real insight into how the system works.
"letting it go out of scope" "yes, I guess that would work" tells anyone who reads it that you don't understand how Java works... setting things to null to clears caches? I very much doubt you ever though not keeping your strategies in a list; no example to copy and paste. But you are an "expert" so you could not tolerate me being right - and genuinely helpful..

I have listed places where things could be fixed on a couple of occasions and was basically told to do it myself. I will not try to "help" you ever again. What would you do, anyway? Nothing.

@nimo23
Copy link
Contributor

nimo23 commented Jan 15, 2022

@johnthetireless This is just another example of where instead of addressing the issue, you're again distracting and being rude. Me and the community are here to help and spend time to make this project better.

there are so many easy fixes* that you or anyone could make to this library to clean up the array allocations its boggles my mind..you are stuck with some very bad code in some places

I asked you politly: "What kind of (the "so many easy") fixes for what code do you mean exactly?" And what are you doing? Bashing and again not answering any questions.

@nimo23
Copy link
Contributor

nimo23 commented Feb 6, 2022

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Issue describing or discussing an enhancement for this library
Projects
None yet
Development

No branches or pull requests

3 participants