search

betcode-org / flumine

flūmine - Betting trading framework
MIT License
177 stars 60 forks source link

process_new_market is not called in FlumineSimulation #671

Closed mzaja closed 1 year ago

mzaja commented 1 year ago

FlumineSimulation overrides BaseFlumine._process_market_books method, so BaseStrategy.process_new_market does not get called when running a simulation. because the call is not implemented.

Given how similar BaseFlumine._process_market_books and FlumineSimulation._process_market_books, I would refactor the code so that both share as much common functionality as possible. The current code seems to violate DRY principle.

mzaja commented 1 year ago

Pull request opened: https://github.com/betcode-org/flumine/pull/672

I did not do any refactoring myself because I do not know the intricate differences between simulated and live Flumine instances, but I feel that BaseFlumine._process_market_books and FlumineSimulation._process_market_books should be coupled more tightly. The more they diverge, the less confidence there is in simulation results and chances of bugs like these appearing.

liampauling commented 1 year ago

Thanks for this and yes I agree, there is a todo to fix this but from memory I couldn't find a clean way to keep things dry.