Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Clean up options code and better error messages #61

Open
tnelson opened this issue Jan 10, 2021 · 4 comments
Open

Clean up options code and better error messages #61

tnelson opened this issue Jan 10, 2021 · 4 comments
Labels
errors Issues related to error messages refactor Refactoring and code clean-up

Comments

@tnelson
Copy link
Owner

tnelson commented Jan 10, 2021

Right now adding a new option requires changes in several places:

  • the Options struct defn
  • the init-options define
  • state-set-option (in 2 places!)
  • get-option

We did this for 2 reasons:

  • I couldn't figure out how to cleanly update struct values without knowing the field name at compile time (maybe switching to macros could fix this?)
  • It enables some type-checking so students who try to use an unknown field, or give a wildly wrong value, will receive error messages rather than silent failure.

The errors still aren't great because we don't catalogue the actual set of valid values. E.g., logtranslation can meaningfully be 0, 1, or 2 -- but not 100.

@tnelson tnelson added refactor Refactoring and code clean-up errors Issues related to error messages labels Jan 10, 2021
@tnelson
Copy link
Owner Author

tnelson commented Jan 10, 2021

Also note word-separation is inconsistent. Some options run words together (logtranslation) and others use underscores.
Dashes are unfortunately not usable in identifiers in #lang forge, since - means minus and we decided not to adopt Pyret-style whitespace.

@tnelson
Copy link
Owner Author

tnelson commented Jan 10, 2021

Maybe it'd have been better to have an option 'temporal, an option 'target etc. rather than one 'problem-type option. Discuss.

@sorawee
Copy link

sorawee commented Jan 10, 2021

I couldn't figure out how to cleanly update struct values without knowing the field name at compile time (maybe switching to macros could fix this?)

That sounds like a wrong way to use struct... If it's quick to elaborate the problem, could you do that? But if it will take long time, it's probably not worth it then.

@tnelson
Copy link
Owner Author

tnelson commented Jan 10, 2021

We have a struct Options that comprises all the Forge options (core minimization setting, etc. etc. etc.)
To change an option, we call struct-copy. E.g., (struct-copy Options options [solver value]), where options is the identifier for the current options struct, solver is a field name, and value is an identifier bound to the new value.

The problem is that I can't call (struct-copy Options options [key value] where key is an identifier bound to a field name. There's no field named key!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
errors Issues related to error messages refactor Refactoring and code clean-up
Projects
None yet
Development

No branches or pull requests

2 participants