ctubbsii opened a new issue, #1124:
URL: https://github.com/apache/fluo/issues/1124
Currently, `TestTransaction` implements TransactionBase, but still has a
`commit()` and `close()` method with the same signature as the `Transaction`
interface, which itself extends `AutoCloseable`. It should implement
`Transaction` instead.
`TestTransaction` also has a `done()` method that does:
```java
try {
commit();
} finally {
close();
}
```
Once `TestTransaction` is `AutoCloseable`, inheriting that from
`Transaction`, the calls to `done()` can, and should, be converted to calls to
`commit()` at the end of a try-with-resources block, and the `done()` method
should be removed. This will make the lifecycle management of test transactions
in the test code more clear, rather than relying on the implied `close()`
method inside the `done()`. It will also make the order of `commit()` calls
more clear when writing/maintaining test code. With `commit()` hidden inside
the `done()` method, it's not immediately obvious when they are called.
This proposal would cause code written like:
```java
TestTransaction tx = new TextTransaction();
// ... stuff here
tx.done(); // implied commit() and close()
```
to be written more explicitly as:
```java
try (TestTransaction tx = new TextTransaction()) {
// ... stuff here
tx.commit(); // explicit commit()
} // auto closed
```
This example doesn't look that much better, but it's a big improvement when
there are multiple transactions being created, committed, and closed in a test.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscr...@fluo.apache.org.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
