[Design] Decorator pattern for Composite move order

[KYLChiu]

I am more and more convinced a decorator pattern is what we want for our problem.

We can discuss further offline? I am keen to use this pattern - in fact quite keen to work on this PR, but as you know, I probs can only work on this say Weds onwards, don't want to block you.

A youtube reference on decorator pattern, with example code in Java.

Can you elaborate why? I don't see any pros/cons listed in your first link, so don't see why this isn't bad for our case. In the youtube video, it mentions that a benefit could be that behaviour can be changed at runtime. I don't believe we need that here. We don't need to alter the MVV_LVA matrix, nor do we need to alter the conditions or evaluate of CompositeHeuristic. In fact all Heuristics adhere to the base class interface. So how does it help?

Moreover, why we are redesigning after the implementation? I thought this was discussed before. If it does prove hard to extend, I am happy to change (or if you want, for you to change). But perhaps this could be considered in the future?

[yibeili]

had a look at the decorator pattern and the new compositeheuristic class. I wonder in the future, when we add more move ordering method, do we simply add the evaluation score up from all the ordering methods? (i thought it might be more complicated than this...)

if the answer is yes (and if i understand things correctly), i think i see jlo's point and think decorator could work? but seems to me we can also extend to future method by adding to return mvv_lva + killer_move i have no idea about the part that behaviour can be changed at runtime, so not sure about it.

[KYLChiu]

had a look at the decorator pattern and the new compositeheuristic class. I wonder in the future, when we add more move ordering method, do we simply add the evaluation score up from all the ordering methods? (i thought it might be more complicated than this...)

if the answer is yes (and if i understand things correctly), i think i see jlo's point and think decorator could work? but seems to me we can also extend to future method by adding to return mvv_lva + killer_move i have no idea about the part that behaviour can be changed at runtime, so not sure about it.

@yibeili @ccjeremylo

Adding up the evaluate scores is just one way to do it. If you think of a better way - do suggest! The idea is that if we have multiple heuristics that apply to the same move, we get both benefits:

if non_capture:
   score += killer_move(move)
   score += history(move)

Btw I actually don't know how to implement this with decoration. With a naive implementation, suppose you just return whatever the singular function does on top of the previous ones, e.g. composite_score = killer(mvv_lva(move)). Then because killer is conditional on non-captures, suppose we do get a capturing move - do we return 0 directly without going to mvv-lva logic? In the current code, because we aggregate, we get to run both functions.

Anyway, perhaps it could work, but no one's telling me why it improves on the current design! I'm happy for it to change in the future, but if there's no concrete benefit - why do it? It would be nice to see a little POC or code demonstration to see how we plan to do it (if it happens).