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

Refactor dereference operations into tagged unions #1603

Merged
merged 8 commits into from
Jan 9, 2024

Conversation

distributivgesetz
Copy link
Contributor

@distributivgesetz distributivgesetz commented Jan 2, 2024

It's just like rust enums no wayyyy

This PR tackles mainly nullability by refactoring the existing Dereference operation to be safer with the help of the type system.

The previous implementation used a struct called Operation, which held an enum value for what kind of operation the struct described as well as metadata about the operation. This resulted in a struct where pretty much every additional metadata was optional and had to be asserted at the time of checking Kind, which wasn't very elegant.

I refactored dereference operations by binding the metadata into types with one shared superclass called Operation.
The subclasses are there to describe all kinds of different metadata that they need. To get the metadata, you can use type patterns to extract the data. This completely eliminates the need to assert no nullability with !, making the code much clearer and more elegant.

Since Operation had to inherit from other things now, I had to turn it into a class. This has zero impact on performance since all operations were located on the heap anyways.

Additionally I cleaned up some of the code involving dereferences. One less area with warnings.

@distributivgesetz distributivgesetz marked this pull request as ready for review January 3, 2024 10:39
@wixoaGit wixoaGit merged commit cf334d6 into OpenDreamProject:master Jan 9, 2024
6 checks passed
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.

2 participants