Closed kant2002 closed 6 years ago
So does writing down exact values actually provide more testability and correctness for BWEM? What if we change the height measurement to another distance like manhattan? I'm not criticizing, just not sure if this is what we want tested since the absolute values should be an implementation detail, no?
The actual writing down does not helps much. If people start using library, better create only new functionality for them and don't break existing usage. If we decide change meaning for the Area, would be better found new representative name for the new concept and describe in tests how it works. When people update, they just update and start using new stuff, and don't think about potential breaking changes.
For example I see messages in the chat where people ask for having Chokepoints to have width. Adding width to existing Chokepoint class means that likely count of Chokepoints changed as well for the maps. That could mean breaking change for the existing users. It would be more wise create new NarowPath concept which will be provided alongside ChokePoint API
Current tests: TileSize, WalkSize and Center is trivial matter. I write them down just to check how it goes. MaxAltitude is actually highlight. That function has very clear meaning. It is easy to verify. And on the synthetic maps I could not reproduce that meaning using paper and pencil. That means either I wrong, or subtle bug in the implementation which just make some values a bit off, by small margin. Still looking on it.
Future tests: Areas is grey area, I don't know what is correct for them and chokepoints. It should be something obivious, for which custom map which exhibit specific property could be created.
Sure, for the synthetic maps I agree that we could have values. But what if we for example in some way change how the edges work, like inclusive instead of exclusive etc. We should maybe look into a test that doesn't depend on specific values but instead just sees if the values are reasonable.
if change would be inclusive/exclusive it could be easily solved by additional method/parameter without sacrificing backward compatibility. Also having explicit test could give us warning that we change something which will affect end user. Anyway if we think that change is reasonable and really improve end-user needs, then tests could be changed. If you want non-deterministic results, I better avoid having tests for such cases, it allow room for improvement, changes, modifications. Still I think many of your concerns could be solved by improving library in backward compatible way. Look at C# and Java, and they have much harder goal then we 😃
/cc @N00byEdge I add test which verify Altitude values for different mini-tiles. That allow me better understand why I see specific number on the plain dirt map. Also it probably the kind of tests which you want in first place
See #10