4
\$\begingroup\$

Basically, I have a list, newtry of blocks, and I want to find a value of catchtype that makes them all return true for block.isCatchExtendable(newhandler, catchtype) or report an error if this can't be done. block.isCatchExtendable returns two values, success and a new type to try on failure.

My current code works, but the control flow is very convoluted, with numerous breaks and elses. I was wondering if anyone could think of a better way to arrange things. Also, these are all my own functions, so I am free to change the interfaces, etc.

while 1:
    ct_changed = False

    for block in newtry:
        success, newcatchtype = block.isCatchExtendable(newhandler, catchtype)
        if not success:
            if catchtype == newcatchtype: #no success and no other types to try, so just break and fail
                break
            catchtype, ct_changed = newcatchtype, True
    else:
        if ct_changed:
            continue
        else:
            break
    error('Unable to extend try block completely')

isCatchExtendible:

def isCatchExtendable(self, newhandler, catchtype):
    return self.catchset.extendible(newhandler, catchtype, self.currentHandlers)

This then calls:

#If not extendible with current catchtype, returns suggested type as second arg
def extendible(self, newhandler, catchtype, outerhs):
    if catchtype is None:
        temp = self.catchsets.get(newhandler)
        if temp is None:
            return True, None
        return False, temp.getSingleTType()[0]

    proposed = ExceptionSet.fromTops(self.env, catchtype)
    inner = [h for h in self.handlers if h != newhandler and h not in outerhs]
    outer = [h for h in self.handlers if h in outerhs]

    sofar = ExceptionSet.fromTops(self.env)
    for h in inner:
        sofar = sofar | self.catchsets[h]
    if (proposed - sofar) != self.catchsets[newhandler]:

        #Get a suggsted catch type to try instead
        suggested = (self.catchsets[newhandler] | sofar).getSingleTType()
        suggested = objtypes.commonSupertype(self.env, suggested, (catchtype, 0))
        assert(self.env.isSubClass(suggested[0], 'java/lang/Throwable'))
        return False, suggested[0]
    for h in outer:
        if not proposed.isdisjoint(self.catchsets[h]):
            return False, catchtype
    return True, catchtype
\$\endgroup\$
2
  • \$\begingroup\$ Could you share the code or algorithm for isCatchExtendable?, I suspect a better interface for that could help here. But I don't know what its doing. \$\endgroup\$ Aug 12 '12 at 15:31
  • \$\begingroup\$ @Winston I added the code for isCatchExtendable. The basic idea is to determine whether adding another catch handler to a try block will preserve the semantics. \$\endgroup\$
    – Antimony
    Aug 12 '12 at 16:01
3
\$\begingroup\$

I think your algorithm suffers from awkward splitting of code. The suggestion logic really should be part of your algorithm, not hidden in some other code.

If I'm understanding correctly, isExtendable really considers three different sets of exception classes

  1. May Catch: These exceptions are caught by handlers before the one of interest, it is safe to catch these as they will already be caught
  2. Should Catch: These are the exceptions which the given handler should be catching
  3. Must Not Catch: These are the exceptions which handlers after the one of interest catch. These must not be caught, as that would prevent the current handler from catching them.

We want to pick a single exception type that fulfills the requirement. It must catch everything in Should Catch, some subset of things in May Catch, and nothing in Must Not Catch.

We can combine all May Catch exceptions by taking their intersection across all blocks. Its only safe to catch them if they will already have been caught across all the blocks.

We can combine all the Must Not Catch, by taking their union across all blocks. The should catch, I assume, is the same across all blocks.

Hence your algorithm looks something like:

may_catch = intersection(block.may_catch for block in blocks)
may_not_catch = union(block.may_not_catch for block in blocks)

for catch_type in catch_types.iterparents():
    will_catch = catch_type.catchset()
    if will_catch - may_catch ==  should_catch and will_catch.isdisjoint(may_not_catch):
        break # found it
else:
    error('no type suffices')
\$\endgroup\$
1
  • \$\begingroup\$ Nice idea. I didn't think of taking the intersections across multiple blocks. \$\endgroup\$
    – Antimony
    Aug 13 '12 at 15:37
6
\$\begingroup\$

First off, a pet peeve of mine: while 1 makes no semantical sense. You want while True.

However, in your case you actually want while ct_changed:

ct_changed = True
while ct_changed:
    ct_changed = False
    for block in newtry:
        success, newcatchtype = block.isCatchExtendable(newhandler, catchtype)
        if not success:
            if catchtype == newcatchtype:
                break
            else:
                catchtype = newcatchtype
                ct_changed = True

Alternatively, you can flatten the nesting level by inverting the if not success conditional, and continuing:

        …
        if success:
            continue

        if catchtype == newcatchtype:
            break

        catchtype = newcatchtype
        ct_changed = True

(In fact, I’d probably go for this.)

\$\endgroup\$
1
  • \$\begingroup\$ How do you detect the error condition then? Edit: Nevermind, you can just put the error immediately after the if catchtype == newcatchtype instead of breaking. \$\endgroup\$
    – Antimony
    Aug 12 '12 at 16:05

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.