Exception Handling & Dereference styles

[Eric Dalquist]

I'd like to make a proposal for some changes to the internals of the
Cernunnos code base. Functionaly there should be little to no change but
many of the files would end up having fairly large diffs because of the
refactoring. There are two parts:

1) Tightly scope exception handling. Many Tasks and Phrases have one big
try/catch block that catches a Throwable. While this works it can get
confusing as many times the block includes many calls that could throw
an exception and the outer message doesn't always correctly describe
what failed. Reducing the try/catch blocks to just include code that
actually require them (checked exceptions or unchecked exceptions that
can be handled) and providing operation specific error messages can help
greatly with debugging a problematic CRN script. Also since the
decorators always wrap every task and phrase a try/catch around the
entire logic of the task/phrase usually just adds an extra layer in the
stack without much value.

2) Reduce usage of multiple dereferences on a single line. I've had to
debug a few things in CRN that have been difficult to track down due to
one line looking like obj1.getFoo().doSomething().add(bar); That one
snippet has 4 opportunities for exceptions to be thrown and only one
line number to reference in the stack trace. Using local variables costs
nothing in these cases and makes debugging code much easier.

I've attached an example of refactoring the AddGrammarTask for both of
these issues. I would be more than happy to take this on for other tasks
when I get time here and there. Functionally this patch shouldn't change
the behavior of AddGrammarTask, only change what the stack traces look
like if an exception is thrown and provide better line number
information when debugging an exception.

-Eric

[Eric Dalquist]

I just realized I had made a small change in SimpleEntityConfig that
would be useful for that patch to actually compile one applied.
Attaching an updated patch

-Eric

[Andrew Wills]

Eric,

Ack -- I had a thorough response typed up that I lost to a browser
crash. I thought gmail auto-saved my drafts... :?

#1
---
All of this sounds fine. The try/catch blocks in the decorators are
newer than many of the tasks, so I guess it makes sense to rethink
exception handling w/in tasks somewhat in light of these capabilities.

I'm most excited about doing what we discussed yesterday: moving the
super.performSubtasks() calls to outside the try/catch. I'm less
motivated to work through the set of tasks, proactively changing them
to catch only checked exceptions, etc. I'd be happy to update them
one-by-one as the opportunities come up (i.e. when I'm already
enhancing them somehow), and you're certainly welcome to tackle them
if that's your priority.

#2
---
Code like the following:

obj1.getFoo().doSomething().add(bar);

has some important advantages over the alternative:

import com.my.Foo;
import com.my.Bar;

final Foo foo = obj1.getFoo();
final Xyz xyz = foo.doSomething();
xyz.add(bar);

Just now, I'd prefer *not* to establish a policy preference for the later.

But on the other hand, I've also encountered ambiguous NPEs from code
like the former. So how about this plan: should you (or anyone)
actually run into a problem with a line of code that looks like the
former, then by all means change *that line* to prevent the issue in
the future. My sense is that some chained expressions are susceptible
to issues (particularly w/ NPEs) and some are much less so.

Does that sound good?

drew

[Eric Dalquist]

For #2, what are some of the advantages? While the imports aren't explicitly listed in the single line version the compiler still does the same checks and actually provides more confusing messages if a class can't be found on the single line version versus a missing import. It isn't a big issue and if you favor the single line version thats just fine, I'm just curious as to what other advantages multiple dereferences on a single line provide.

For the #1 work I'll probably tackle a few here and there. I'd love to have the exceptions cleaned up for the 1.1 release so debugging scripts becomes a bit easier.

-Eric

Andrew Wills wrote:

--~--~---------~--~----~------------~-------~--~----~
You received this message because you are subscribed to the Google Groups "Cernunnos Discussion" group.
To post to this group, send email to cernunnos-...@googlegroups.com
To unsubscribe from this group, send email to cernunnos-discus...@googlegroups.com
For more options, visit this group at http://groups.google.com/group/cernunnos-discussion?hl=en
-~----------~----~----~----~------~----~------~--~---