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

Fix create statement #710

Merged
merged 11 commits into from
Oct 19, 2023
Merged

Conversation

jmilkiewicz
Copy link
Contributor

@jmilkiewicz jmilkiewicz commented Oct 19, 2023

Fixing #709

Note on implementation:
As of now createStatement can return one of 3 possible options:

  • success (so statement is created)
  • no session (in case one wants to create a statement for non existing session) - before a nasty 500 was thrown
  • statement shall not be created as session is "completed"

To model a function/method which can return one of these 3 options (with each of them having complete different shape) i see/know following approaches:

  • using multiple exceptions: i do not like this approach as it is controlling a flow with exceptions. Beside, these 3 cases are not exceptions, they are just regular sunny/rainy-day outputs.
  • using one common, super set class which can host all data of these 3 options: code is very nasty with if/else (as at some point i need to figure out what option is actually retuned and act accordingly), super-set class is often very low-coherent
  • using algebraic data types with pattern matching- it is IMHO the best approach, but it is not available out-of box in "old" java

I decided to go for option 3 but as we are in old java, without kotlin/scala the implementation is pretty exotic as it is based on visitor pattern, ie I return StatementCreationResult which can be visited with StatementCreationResultMapper. Both are abstractions and the overall solution provides:

  • exhaustive type checking (by using visitor), so in case code needs to be extended i will provide a new implementation of StatementCreationResult which would force me to update StatementCreationResultMapper.
  • StatementCreationResultMapper separates logic from "delivery-mechanism" being micronaut here. So high level policy does not depend on low level policy...

If it were new java/kotlin the code would be much simpler, sth like:

sealed interface StatementCreationResult

class StatementCreated(statement: Statement) : StatementCreationResult
object NoSessionExists : StatementCreationResult
class SessionInInvalidState(invalidState: ApplicationState) : StatementCreationResult 

and in controller

val result = sessionService.createStatement(id, statement)
when(result) {
is NoSessionExists ->  HttpResponse.notFound()
is SessionInInvalidState -> HttpResponse.badRequest("invalid session state " + invalidState);
is StatementCreated ->  HttpResponse.created(statement);
}

Compiler will force me to handle all possible options in when statement and the code will be more straightforward

@pdambrauskas pdambrauskas merged commit 8924a6b into exacaster:master Oct 19, 2023
2 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