Either renaming or dropping the Sink.Calc method

[oleiade]

Feature Description

In the context of #2755 resolution, we have found out with @imiric that the TrendSink.Calc method is only implemented for the TrendSink. Furthermore, although it is named Calc, its actual job is to compute the median value of the sink.

Suggested Solution (optional)

We would like to propose some ideas on how to improve that state in the future by either:

• dropping that method from the interface itself to only keep its implementation on the TrendSink
• converting that method in a dedicated helper function and let whoever relies on the median value to call it on the TrendSink.Values
• renaming that method to Median or something along those lines (in line with the P method), for consistency, and to explicit what it's used for.

Already existing or connected issues / PRs (optional)

#2755
#2759
#2320

[oleiade]

@codebien pointed out that this might end up being solved eventually, as a side-effect of #763 👀

[oleiade]

#2759 dropped the Calc method from the Sink interface unilaterally.

@mstoykov suggested that we should further drop the Med attribute of TrendSink. Further, considering that most calculations involved in dealing with the TrendSink involve calculations, he suggested we should design the Sink/TrendSink APIs to compute those values "just in time", when they're actually used (by thresholds for instance).

Those changes should be implemented somehow before we can consider this issue done.

[mstoykov]

I think we can close this @oleiade ?

[oleiade]

I wanted to keep it open as a reminder to address calculating avg, min, max, etc "just in time" during cooldown. Which we haven't addressed yet. What do you think?

[na--]

Hmm wouldn't calculating these "just in time" actually be a performance degradation? They are currently calculated on every Add() call because that's the easiest and most efficient implementation, not sure how you can improve it 😕

👍 for closing this issue, since Sink.Calc() was dropped by #2759, and this is what the issue is mainly about. We can open new issues for other potential changes, e.g. #2320