Reduce code duplication in track_hit_feedbacks code
There's a lot of duplicated Option<bool> code in the feedbacks. This should probably be moved to a wrapper type.
Hey Addison,
Since I implemented the feedback hit tracking I have a question regarding this issue:
What would be the benefit of having a wrapper type? This will add another layer of complexity IMO, beneficial only to a very limited subset of Feedbacks.
From my understanding there are 3 different types of feedbacks:
- Composite (
feedback_and,feedback_or) - Static (feedbacks to annotate metadata for the testcase)
- Compute (feedbacks that actually compute data from provided by observers)
Only Computed feedbacks will actually need to cache their results and IMO it makes sense for them to just store last_result in the struct (as they do already). Not sure what static and composite feedbacks have to gain from caching.
I think the duplicated code here is fine since it's just one function.
Makes sense. I think, when I was working on #2438, I was finding lots of duplicated code relating to hit tracking. That is beyond the last_result value in the struct; it's all of the trait changes that the functionality requires.
We can leave it for now, but it feels wrong to slam this into the feedback trait. Maybe we need to revisit how we implement feedbacks s.t. we can somehow add new functionality dynamically.
But the very nature of the feature implies that every Feedback must implement it? But I agree that Feedbacks need a re-visit, as there are too many static Feedbacks and it feels unintuitive