[GitHub] [jena] rvesse commented on a change in pull request #872: JENA-2004 Implement functionality currently provided by Txn class directly in Transactional. Deprecate Txn class.

[GitBox <gi...@apache.org>]

rvesse commented on a change in pull request #872:
URL: https://github.com/apache/jena/pull/872#discussion_r531487296



##########
File path: jena-arq/src/main/java/org/apache/jena/sparql/core/Transactional.java
##########
@@ -192,4 +174,110 @@ public default boolean promote() {
 
     /** Say whether inside a transaction. */
     public boolean isInTransaction() ;
+
+    /**
+     * Execute and return a value in a transaction with the given {@link TxnType transaction type}.
+     */
+    public default <T extends Transactional, X> X calc(TxnType txnType, Supplier<X> r) {

Review comment:
       The downside of `default` methods is that since they become part of the interface implementations could choose to override them especially since you cannot declare them to be `final`
   
   The advantage of the existing `Txn` helper class is that it provides a predictable and consistent behaviour regardless of the `Transactional` implementation it is operating over i.e. if you are using `Txn.executeRead()` it's always going to behave the same whereas calling `ds.executeRead()` could change behaviour depending on the dataset implementation.

##########
File path: jena-arq/src/main/java/org/apache/jena/sparql/core/Transactional.java
##########
@@ -31,32 +33,12 @@
  * <pre> begin(WRITE) ... abort() or commit()</pre>
  * <p>{@code end()} is optional but preferred.
  * <p>
- * Helper code is available {@link Txn} so, for example:
- * <pre>Txn.executeRead(dataset, {@literal ()->} { ... sparql query ... });</pre>
- * <pre>Txn.executeWrite(dataset, {@literal ()->} { ... sparql update ... });</pre>
- * or use one of <tt>Txn.calculateRead</tt> and <tt>Txn.executeWrite</tt>
+ * In most cases instead of calling primitive methods such as {@code begin} and {@code commit }
+ * it makes more sense to use high-level default methods:
+ * <pre>dataset.executeRead({@literal ()->} { ... sparql query ... });</pre>
+ * <pre>dataset.executeWrite({@literal ()->} { ... sparql update ... });</pre>
+ * or use one of <tt>calculateRead</tt> and <tt>executeWrite</tt>
  * to return a value for the transaction block.
- * <p>

Review comment:
       Since the old style will still be supported and used by many users I don't think it makes sense to remove the old style from the Javadoc entirely




----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@jena.apache.org
For additional commands, e-mail: pr-help@jena.apache.org