Hi Alessio,
On Sat, Jan 22, 2011 at 11:12 AM, Alessio Stalla alessiostalla@gmail.com wrote:
On Sat, Jan 22, 2011 at 10:41 AM, Erik Huelsmann ehuels@gmail.com wrote:
Anyway. Some comments, without having looked at the situation in detail:
- disallowing overriding getMessage() feels wrong. The design may, and
likely is, incomplete, but I got a very vague feeling that the function is serving a purpose. Now, I'm not entirely certain whether that purpose exists on the lisp or the java side. I don't have a clear picture of that yet.
Right. I think I should have said "discourage overriding". However, the original getMessage() should be reduced to "return null" - I think, because it's calling writeToString() which itself is calling getMessage() again...
getMessage() is useful when an exception escapes to Java and you want to understand what happened (it's part of the API of Throwable and used to log the exception). Currently ABCL is not very helpful in that regard: if you disable the Lisp debugger to have exceptions propagated to Java (as it's often a case when you use ABCL as a library) you get back anonymous exceptions with no information on what happened. That is, I think, in important area of improvement.
Am I right to think that you'd do so by using Interpreter._DEBUGGER_HOOK_FUNCTION_ ? If that's true, then you're using Interpreter.UnhandledCondition to pass the error on to Java, including its description.
Interpreter.UnhandledCondition uses <condition>.writeToString() instead of using getConditionReport(). At least with my proposed design, I'd advocate changing all calls to condition.writeToString() to condition.getConditionReport() in those cases which are meant for human readable output.
I'd say the above should solve your issue. It did make me realize that it's not Primitives.ERROR which should bind the *PRINT-** variables, but it should be getConditionReport() because it may be invoked in multiple places.
Summarizing: I think getMessage() should be protected; it's a method to change the behaviour of getConditionReport(). The latter should be public, because *that*'s the method which should be used all over the place.
Agreed?
Bye,
Erik.