mixite
mixite copied to clipboard
SatelliteData interface should be optional
As an API interface SatelliteData
is actually a very opinionated choice of data. There is no hard requirement for any enforced supertype to the storage data (e.g. the core class, HexagonalGridImpl
, doesn't use it even once). SatelliteDataexists so Mixite can supply some useful algorithms via the extended
HexagonalGridCalculator. If the user doesn't need
movementCostor
isPassableor
isOpaquethen it's just cruft (note for example
movementCost` is not used anywhere in the code base!).
Ergo, SatelliteData
interface should be optional.
However, those additional utilities are very useful and key part of the value of this library.
first thoughts:
- Remove calculator from builder
- Split single calculator into seperate items based on actual data requirement (several methods don't need
SatelliteData
at all)- It's possible to avoid the interface altogether by supplying a function. E.g.
isVisible( from:Hex, to:Hex, isOpaque: Hexagon->Boolean)
- It's possible to avoid the interface altogether by supplying a function. E.g.
So this is some very old code, that's why these weird things are in the codebase. I agree that this needs to be cleaner, but I didn't work on this for a while now so I need to wrap my head around it in order to be able to respond to this. What we could do is to have multiple interfaces and one of them has Nothing
in place of the satellite data class. I'll take a look probably at the end of this year. How would the split look like in your opinion?
Nor sure as I've only been a Kotlin programmer for about 4 hours (literally). Theres a lot of nice 'infix' pimp-my-api stuff could be done but I don't know how that works in mpp etc. So much to learn!
It is a good idea, but we have to maintain API compatibility with Java. 😢
I don't think it's mutually exclusive. if the calculators are simple standalone classes then a "clever" infix layer that glues a them onto suitable Hexagon<T>
is an addition for Kotlin users. Too much for me to comment more on till I've gotten the hang of KT a bit more but what I will PR at some point is removing T: SatelliteData
from all classes except that calculator.