venasolutions / bosk

Apache License 2.0
11 stars 4 forks source link

Simplifying BoskDriver with optional replacement values #22

Closed prdoyle closed 10 months ago

prdoyle commented 1 year ago

Background

BoskDriver has five update methods:

  1. submitReplacement and submitDeletion
  2. submitConditionalReplacement, submitConditionalDeletion, and submitInitialization

Early, we considered using submitReplacement(ref, null) to represent deletion, but because null is a very common symptom of programming mistakes, we wanted to make deletion more explicit.

Problem

Five methods to implement updates with slightly different semantics seems a bit much. It's a burden on implementers, who often end up with five copies of a bunch of code.

Solution

The notion is to use another null-like value that is not null. The challenge is: this is not generally possible, because null is the only value in Java that can be passed for any (non-primitive) type.

One idea would be that the value argument of submitReplacement would be an Optional or similar.

We would then write the updates like this:

We could do the same for the requiredValue of submitConditionalReplacement, and by passing Optional.empty() for the requiredValue, we then would no longer need submitInitialization.

This would reduce the replacement methods to two: submitReplacement and submitConditionalReplacement.

Transition

We could even keep the current five methods as default methods in the BoskDriver interface, so users can call them for convenience, so they don't need to use Optional if they don't want to.

gradycsjohnson commented 1 year ago

Would it be better to make the new submitReplacement(ref, Optional.of(val)) default and have it call submitReplacement(ref, val) or submitDeletion(ref) depending on if newValue.isPresent()?

prdoyle commented 1 year ago

@gradycsjohnson - I just saw your question, sorry for the delay!

The attractive thing about using Optional is that it would mean driver implementations need only implement two submit methods instead of five. If we add the Optional versions as default methods on the interface and leave the existing five methods unimplemented, we don't get this benefit.

However, it's a fine first step if you want to start with that!

gradycsjohnson commented 1 year ago

Right! Sure I'll go back to the first approach :)

prdoyle commented 1 year ago

To be clear... your way is much less effort, and is not a breaking change. If you wanted to start with that, I'm onboard.

If you're up for doing the whole thing, that's awesome too! It will affect every BoskDriver implementation in the codebase (in a good way! That change will probably delete much more code than it adds).

prdoyle commented 10 months ago

I'm feeling like this might not be an improvement after all. The proliferation of Optional arguments seems to have its downsides, and really, five methods instead of two isn't really that many.