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

Check usage of var vs val in all schemes #476

Closed
FWDekker opened this issue Oct 2, 2023 · 1 comment · Fixed by #483
Closed

Check usage of var vs val in all schemes #476

FWDekker opened this issue Oct 2, 2023 · 1 comment · Fixed by #483
Assignees
Labels
code quality Code changes without behavior changes
Milestone

Comments

@FWDekker
Copy link
Owner

FWDekker commented Oct 2, 2023

For non-primitive types, anyway. Check that they are vals wherever possible. I think the serialisation doesn't like it when a list is a val instead of a var, but honestly, tough luck! I really prefer using a val there. I really dislike mutability. But do a sanity check anyway whether my dislike is justified there.

@FWDekker FWDekker added the code quality Code changes without behavior changes label Oct 2, 2023
@FWDekker FWDekker added this to the v3.0.0 milestone Oct 2, 2023
@FWDekker FWDekker self-assigned this Oct 2, 2023
@FWDekker
Copy link
Owner Author

The following Lists are notable and should definitely be vals rather than vars:

Class Field Annotation
Settings templateList @OptionTag
TemplateList templates @MapAnnotation
Template schemes @XCollection

FWDekker added a commit that referenced this issue Oct 19, 2023
The goal was actually only to fix the usages of `val`s and `var`s across the `Scheme`s (cf. #476), but this ended up in being a major overhaul of how the various `Tree`-related classes work together. I found quite a few pieces of code that were, by themselves, neat, but for which much better solutions existed. With these changes, I feel more confident in the stability of the code, and plenty of docs and tests have been added as well. I am aware that #473 will be another overhaul of the `Tree`s, but that won't be until v3.1.x, and I don't want to wait for that. Also, depending on the solution for that issue, the code here might be retained completely anyway.
@FWDekker FWDekker mentioned this issue Oct 19, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
code quality Code changes without behavior changes
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant