Skip to content

Validate settings read from disk#21

Merged
ways2read merged 6 commits intomainfrom
VerifySettingsReadFromDisk
Apr 1, 2022
Merged

Validate settings read from disk#21
ways2read merged 6 commits intomainfrom
VerifySettingsReadFromDisk

Conversation

@ways2read
Copy link
Member

Add function to check that preference settings are present and valid, use defaults if not.

Resolves #17

Copy link
Collaborator

@NSoiffer NSoiffer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems clunky to keep resetting valid_test_passed and repeating the same structure over and over. I think a better solution is to have a function

validate(user_prefs, keys, valid_values, default_value)

where keys are a list of indexes and valid_values is a list of valid values. Then you can just have

validate(user_preferences, ["Braille", "BrailleCode"], ["Nemeth", "UEB"], "Nemeth")

and have that repeated over and over (maybe boolean could be with [True] or maybe a separate function for that. That would substantially reduce the amount of code.

The implementation of validate would be similar to one of the chunks you currently have for each condition (although it would involve loops or mapping over the keys.

In some languages, it is possible to pass a pointer to the value you want to set (e.g., &user_preferences["Braille"]["BrailleCode"]) which allows you to both dereference it to get the value and also to set it. I wouldn't be surprised if there were a clever way to do that in python, but I don't know it off the top of my head. That would allow you to combine the first two args and avoid a loop in the implementation to get to the dictionary value)

@ways2read
Copy link
Member Author

Agreed, this is clunky and repetitive. I'm not sure I can use exactly the approach you suggest since I think it may throw an exception before I am ready to catch it. But I have enough of a pointer to go on (pun intended!).

@NSoiffer
Copy link
Collaborator

I think by moving it into a function, you can do the catch there and do the correction there (which is why I suggested added the default value to the call).

@ways2read ways2read merged commit b2d36d8 into main Apr 1, 2022
@ways2read ways2read deleted the VerifySettingsReadFromDisk branch April 5, 2022 09:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Bad value in prefs.yaml causes MathCAT settings to fail

2 participants