Aklakan commented on code in PR #3441:
URL: https://github.com/apache/jena/pull/3441#discussion_r2386339028
##########
jena-rdfconnection/src/main/java/org/apache/jena/rdflink/RDFLinkHTTP.java:
##########
@@ -221,163 +220,54 @@ public Graph queryDescribe(String queryString) {
public boolean queryAsk(String queryString) {
return
Txn.calculateRead(this, ()->{
- try ( QueryExec qExec = query(queryString, QueryType.ASK) ) {
+ try ( QueryExec qExec = query(queryString) ) {
return qExec.ask();
}
} );
}
- /**
Review Comment:
> The QueryType may not be particularly useful in this impl
The main reason is that I though that `QueryExecBuilder` should be the
single point of truth, and logic outside of it should be avoided.
`QueryExecBuilder` does not have an API to which the query type can be passed,
and in these specific cases, the `QueryExec` created by the `QueryExecBuilder`
attempts to execute the string with a certain query type just one line below -
so it felt redundant.
Of course, one could argue that e.g. in the future `QueryExecBuilder` could
receive e.g. `query(queryString, queryType)`. Then the query type here could
eventually passed to the extended builders. But as these specific methods are
shortcuts with consistent use - the appropriate exec method, such as `.ask()`
is just one line below - it wouldn't add anything here.
So I couldn't find good arguments for why to have this extra complexity in
the shortcuts.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
