GeezerVison

I have run the format plugin to sort out the whitespace and removed the separators for static inner classes (personally I think they improve readability but appreciate they are not everybody's cup of tea). Not a fan of some of the decisions that the formatter is making, but formatting is incredibly subjective so having automated opinionated formatting does make life easier in a shared code base.

We just use the Spotify fmt-maven-plugin which implements the Google Java Style (which is prescriptive and has no configuration options). I used to use PMD and Checkstyle on LmdbJava but I found it problematic because (a) every update would introduce new warnings which had to be suppressed for no apparent benefit and (b) they reflect options which are debatable at best and impeded people who wanted to provide PRs. I think the code formatter's prescriptive and automated approach strikes the right balance.

While this PR adds the Dbi builder, I decided to keep these methods for the really simple cases. The builder can be used for everything else more complicated. Hopefully a reasonable compromise.

  public Dbi<T> openDbi(final byte[] name, final DbiFlagSet dbiFlagSet) {
  }

  public Dbi<T> openDbi(final String name, final DbiFlagSet dbiFlagSet) {
  }

The builders are such a significant ergonomic improvement and given we are targeting 1.0.0 it might be a good opportunity to remove those methods to encourage visibility of the new golden path.

I have removed the ByteUnits dependency as that repo is no longer being maintained and we were only using a fraction of it and only in tests. I have replaced it with our own ByteUnit class and added new overloaded setMapSize methods to Env and Env.Builder to make it easier for users to specify larger sizes.

Excellent.

@benalexau

The builders are such a significant ergonomic improvement and given we are targeting 1.0.0 it might be a good opportunity to remove those methods to encourage visibility of the new golden path.

Originally I had deprecated all of the open( methods so the only route for Dbi creation was the builder, however I then started to think that for the simplest cases where you just want a named db with no special options, it would be worthwhile to keep the simplest of the open methods. i.e.

        env.createDbi()
            .setDbName("foo")
            .withDefaultComparator()
            .setDbiFlags(DbiFlags.MDB_CREATE)
            .open();

vs

env.open("foo", DbiFlags.MDB_CREATE)  // where MDB_CREATE is treated as a DbiFlagSet

The open(byte[], DbiFlagSet) variant can probably just be done via the builder as that seems like a less common use case.
Having this one open(String, DbiFlagSet) method may also reduce the amount of deprecation warnings people will see vs if it was deprecated.

I'm not too bothered either way so happy to go with whatever you decide.

        env.createDbi()
            .setDbName("foo")
            .withDefaultComparator()
            .setDbiFlags(DbiFlags.MDB_CREATE)
            .open();

vs

env.open("foo", DbiFlags.MDB_CREATE)  // where MDB_CREATE is treated as a DbiFlagSet

I'm not too bothered either way so happy to go with whatever you decide.

I prefer the builder approach:

• Explicitness: The builder shows that foo is the database name, a default comparator exists, and multiple flags are available. Compare that to env.open(..), which provides no such indication (DbiFlags.MDB_CREATE looks like a mandatory constant rather than part of a configurable set)
• Ergonomics: When users encounter unexpected iteration order, they'll recall the comparator configuration. The builder guides them through compatible comparator selection, avoiding confusion about whether comparators exist, where they're configured, or key formatting (eg endianness, string padding, encoding etc)
• Easier support: Builder-based code in tickets clearly shows the exact configuration and saves a communications round-trip asking for the precise Dbi configuration
• Clean migration path: Given we are planning the next release to be 1.0.0 anyway, we can reasonably deprecate these ~10-year-old methods

@benalexau Works for me. Builder it is. 👍