Skip to content

Refactor #76

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

Merged
merged 58 commits into from
Feb 7, 2025
Merged

Refactor #76

merged 58 commits into from
Feb 7, 2025

Conversation

Orciument
Copy link
Member

I want to pretty majorly refactor the current bot.
The main goal is to improve stability and increase the ability to reason about the bot as a system, and to remove some features that were prototyped already but are now dropped.

How this should be done is via organising the code based on how 'safe' it is.
More flaky code goes into a special folder, and gets an API that clearly states all the failure conditions. I think something like this necessary because a lot of the Java ecosystem has hidden failure modes (most often RuntimeExceptions) and protecting yourself against all of these hidden failures everywhere is hardly possible.

So the idea is to put all of this flaky code with hidden failure modes in one place, and only export safe (with all failure modes clearly stated) API's to the rest of the application.
The benefit is that you can reduce the mental load because you have one place where you really need to focus on what you are doing, and if there are any additional failure modes. This idea draws inspiration from Rust's unsafe {} block.
I don't think we will be able to eliminate all unexpected behaviour, but I think we should be able to reduce it, and make it easier to find the issue.
This special directory would also include most IO code because it is inherently messy, and no specifications of outside parties should be fully trusted.

This split would also make it pretty easy to have different standards when it comes to testing. Testing IO code is significantly harder compared to a normal function, so you may need to make some compromises or use manual testing instead.

(I mean IO not on a low-level level but more generally as anything where we are interacting with something outside this application, like a database)

The following features will be removed:

  • All Alerts besides Donation alerts (these are no longer wanted by clym)
  • dynamic function/class registration (via the @input annotation), such dynamic behaviour at runtime, is not without cost. And since we are not planning for plugin support any more, this slight decrease in DX is worth the decrease in complexity. This also means that the modules folder will be removed, and everything that is still needed will be moved. (@subscriber will be retained for now)
  • Donation Goal prototype. This feature will only come later, and the current prototype only really contains the Object definition, so it will temporarily be removed to reduce clutter

…eporter, make HealthManager work for things other than Inputs
… getNestedReplacement to use VarStatement to preserve invariance, adds FieldDoesNotExistException
… remove wildcard destructuring support in loops, add test environment for fuzzing
@Orciument Orciument self-assigned this Feb 2, 2025
@Orciument
Copy link
Member Author

I can give a bit of an update to the sections about classifying the code by "safety":
What i actually rather mean is "predictability", the thing I was actually concerned about were unchecked (-> Runtime) Exception, because they are often not specified in the function signature.
My problem with it is that they unwrap the call stack in a way you do not expect, because you didn't even know they are there. This leads to bad errors, because we might semantically know what the actual bussiness requirement was that caused the error. And it might also lead to erroring from conditions that were recoverable, or returning the wrong errors. Or it could even destroy some of your invariance because it uses a different control flow and might leave your objects in an unknown state.

My solution to that would be to relatively frequently try-catch at points where we would definitely would want to stop any exception, even if no exceptions are specified in the called functions!
This restores your ability to reason to a large degree, because unchecked exceptions are quickly transformed into checked exceptions, and their control flow is then actually explicitly in the code.

@Orciument Orciument merged commit b4b8cde into master Feb 7, 2025
2 checks passed
@Orciument Orciument deleted the refactor branch February 7, 2025 12:15
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.

1 participant