stevedlawrence commented on code in PR #1521:
URL: https://github.com/apache/daffodil/pull/1521#discussion_r2276604630
##########
daffodil-core/src/main/scala/org/apache/daffodil/lib/iapi/Diagnostic.scala:
##########
@@ -128,8 +128,6 @@ abstract class Diagnostic protected (
*/
override def getMessage(): String = message
- override def getMessageOnly: String = message
Review Comment:
Thinking more about this, and looking for other examples, consider
`Throwable` as a reference. `Throwable.getMessage` returns just the message and
`Throwable.toString` returns the message + context (i.e. class name). As
another reference, I found the
[javax.tools.Diagnostic](https://docs.oracle.com/javase/8/docs/api/javax/tools/Diagnostic.html)
class that is also similar. That interface defines a `getMessage` function to
get just the message, and then defines other functions to get other pieces of
diagnostic context. It doesn't also define a function to get everything as a
single string, because that's the purpose of the `toString` function.
So to be consistent with Java, I think we do really want
`Diagnostics.getMessage` to return just the message portion and no additional
context, and `Diagnostic.toString` can return the full mode + message + context
as a string in human readable form.
These examples also feel like an argument against `getMessageVerbose`. In
the java ecosystem, if you want a human readable version of a class and all its
components (e.g. mode + message + context for our Diagnostic class), then
that's what `toString` is for. That's well understood in Java, so adding a
function that is essentially an alias for toString probably doesn't add much
and could possibly cause more confusion (i.e. why call `getMessageVerbose` when
I could just call `toString`?)
So after doing some research, I think I'm leaning towards the original PR as
is. And we recommend that if you want just the message, use `getMessage`. If
you want the entire diagnostic as a string, then use `.toString()`. It might
require changes to API users, but it's a pretty simple suggestion for the
migration doc, and makes our API more consistent with Java conventions.
--
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]
