score combines validation and manipulation. if a score isn't valid, nothing happens which makes anything that uses score harder to understand and can hide bugs

names like score make it sound like a score will happen. if a score might happens, then a name like maybe-score is more appropriate, but I would still suggest separating validation and the actual update

Not sure I understand, are you suggesting moving this

(and (contains? scores->points s) (score-allowed? current-scores s)

another subtle design issue is that the model on the one hand is using constructs like qualified keywords, but the functions in the namespace is accepting strings. basically, this is leaking the fact that the model is expecting to be driven from a command line. I would either change the model to just use strings as keys or I would change the functions to accept keywords and push the type coercion back to the core namespace driving the command line interface.

Thanks!

I would validate in a separate function and I would change the score function to either expect valid data or add a pre condition/assert that throws an exception when invalid data is provided

I personally prefer working on systems that fail fast

in core, I would have something like:

(if (valid? state my-cmd)
   (score state my-cmd)
   (handle-error state my-cmd)) 

What about a test to see if the call to do-score or un-score actually modified the game state, if not that would be an error…would that be “better” as it also keeps the model and command line seperate?

or perhaps the game state could also include a reason why the last attempt to update it failed

I don't think the validation state should be a part of the game state

there's more than one way to do it, but functions that do nothing when given an invalid input are a huge pain to work with in my experience

The part I’m having trouble with is that it feels like there are really two separate validations (neither of which I’m doing very well right now). There is confirming that the command line is well formed. Then there is confirming that the model is effected by that command line and if not why not.

yea. I would consider either of these approaches: 1. move all validation to the core name space. the model expects only valid data. have the model format be convenient for command line usage (ie. use strings). be satisfied that this program will probably only ever be a program with a command line interface 2. change the model to be agnostic of usage (use namespaced keywords et all. make consumers of the library also pass namespaced keywords). add validation based in both core and in the model name space. personally, I would just opt for the 1st. this isn't an exhaustive list either, so if you feel like another option is better, that works too! You're closer to the problem and would be a better judge of what is the best fit. I'm just trying to throw out different perpectives

Thanks, great feedback!!