1
\$\begingroup\$

I put the readable version into the gist file -> link

Please ignore the print and comment in the following code.

I often have to write this sort of logic: init the conditions, and deep into loop condition, get the variables in each iteration and break when the condition meets.

As you can see, I get the first line in a region and then loop until the region ends.

The exit condition is while True and (beginning_spaces - start_beginning_spaces).

Are there any better practices in my case?

 def get_exception_region(self, except_region, max_lines_in_exception=100):
        '''
        # get current line number
        row, _ = self.view.rowcol(except_region.a)
        # get next line's starting point
        next_row_starting = self.view.text_point(row + 1, 0)
        # get the whole next line
        next_row_region = self.view.full_line(next_row_starting)
        print(self.view.substr(next_row_region))
        '''

        # Init first setting
        try:

            row, _ = self.view.rowcol(except_region.a)
            next_line_text, next_line_region = self.get_next_line__text_and_region(row)
            prev_line_text, prev_line_region  = next_line_text, next_line_region
            beginning_spaces = self.count_the_beginning_spaces(next_line_text)
            start_row, start_beginning_spaces = row, beginning_spaces

            print "=================Start================="
            print row, beginning_spaces, next_line_text.strip()


            while True and (beginning_spaces - start_beginning_spaces) < max_lines_in_exception :
                row = row + 1
                next_line_text , next_line_region = self.get_next_line__text_and_region(row)            
                beginning_spaces = self.count_the_beginning_spaces(next_line_text)
                print row, beginning_spaces, next_line_text.strip()
                if beginning_spaces > 1 and beginning_spaces != start_beginning_spaces:
                    print "=================Exit================="
                    break
                prev_line_text, prev_line_region  = next_line_text, next_line_region
            return row, prev_line_text, prev_line_region

            pass            

        except Exception as e:
            exc_type, exc_obj, exc_tb = sys.exc_info()
            fname = os.path.split(exc_tb.tb_frame.f_code.co_filename)[1]
            print("[Error]\n", exc_type, fname, exc_tb.tb_lineno)


            raise e
            sys.exit(e)
            #EndOfExcept
\$\endgroup\$
1
  • 2
    \$\begingroup\$ Isn't while True and somecondition same as while somecondition? Am I missing something obvious here? \$\endgroup\$
    – ChrisWue
    Nov 18, 2013 at 8:02

2 Answers 2

1
\$\begingroup\$

Some code is duplicated between before and inside the while loop. You could place that code in a generator:

from itertools import count

def exception_lines(self, except_region):
    startrow, _ = self.view.rowcol(except_region.a)
    for row in count(startrow):
        text, region = self.get_next_line__text_and_region(row)
        beginning_spaces = self.count_the_beginning_spaces(text)
        #print row, beginning_spaces, text.strip()
        yield row, text, region, beginning_spaces

Using the generator, your main loop is simplified:

#print "=================Start================="
lines = self.exception_lines(except_region)
start_row, prev_line_text, prev_line_region, start_beginning_spaces = next(lines)

for row, next_line_text, next_line_region, beginning_spaces in lines:
    if beginning_spaces > 1 and beginning_spaces != start_beginning_spaces:
        #print "=================Exit================="
        return row, prev_line_text, prev_line_region
    prev_line_text, prev_line_region = next_line_text, next_line_region

I have retained to show where they would go in this approach, but commented them out, seconding Michael's remark.

Note that I took the (beginning_spaces - start_beginning_spaces) < max_lines_in_exception condition out. Unless my reasoning is in error, the condition is always true because your break condition triggers first. I am assuming here that max_lines_in_exception > 1 always, and start_beginning_spaces >= 0.

\$\endgroup\$
0
3
\$\begingroup\$

I'm having trouble figuring out exactly what your code is doing. It's called get_exception_region but it appears to do a lot of printing. Is that just for debugging purposes? I'm going to assume that's the case, and thus that get_exception_region returns the row (line number?), text, and region of the line following the exception. Here are my suggestions, given that assumption:

  • Change the docstring to reflect what get_exception_region does, not how it does it. From another angle, describe why someone would choose to call get_exception_region, not what code they would have to write in its place. If there's any code in the docstring, it should show how to call the function, not reminders on functions it may call to implement itself. If you need the latter, comments are more appropriate for that.
  • If you have control over it, the name except_region.a doesn't convey much. Consider using a more descriptive name than a.
  • As mentioned by @ChrisWue, drop the True and from your while condition. At best it does nothing; at worst it distracts readers and slows the program.
  • Consider if rewriting your while loop as a for loop (for row in range(start_row, start_row + max_lines_in_exception): ...) would add or diminish clarity. This would avoid code with a slight smell (row = row + 1), but in this case I'm uncertain whether the replacement would be better.
  • Avoid catching Exception; if you must suppress most exceptions, it's typically better to catch BaseException (at least in python 3). That said, since this code quickly re-raises the exception, this advice is less crucial than usual.
  • Avoid accessing attributes like tb_frame, f_code, and co_filename in a function not specifically geared towards this. Instead for the usage in your exception handler, prefer using tools from traceback such as print_exc; in other cases inspect can be similarly useful.
  • Remove dead code. Having raise e followed by sys.exit(e) is misleading, since the raise will unconditionally prevent the exit from executing. Similarly the pass after return will never be reached.
  • If all you are really doing in this handler is printing out a partial trace, consider omitting the try/except handler entirely, and letting the caller be fully responsible.
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

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