Getting happier ;-), but wondering if I'm thinking pythonically

[Rogier Steehouder]

First time post and my experience with python is hardly extensive. But...

In article <CYvya.128$Eg1.3 at news-binary.blueyonder.co.uk>, 
graham at rockcons.co.uk wrote:
> 
> Firstly, is this in general, the right way?  This is my second attempt - the
> first program worked but didn't use classes, or optik, and was more limited
> in scope.  BTW is using external modules like optik a bad idea if you want
> programs to be portable (doesn't matter for this program, but might)

If you are worried about portability, I think you should confine yourself to 
the standard library. As of v2.3 optik is part of the standard library under 
the name optparse.

> second, I want to use split and a RE to split the fonttable at a pattern,
> hence lines 99-109.  This seems cumbersome - I'd like to do something like:
> 
> for (fnum,fdesc) in re.split(fntid_s,self.buff):
>         add_to_a dictionary

It looks like you are doing something like:
        'ABABABAB' => ['A', 'B', 'A', 'B', ...]
and then picking A and B repeatedly. (Where A and B are more complicated than 
this.)

Maybe re.finditer() would work:

for m in re.finditer('(A)(B)', 'ABABABAB'):
    A = m.group(1)
    B = m.group(2)

>    29           except:         # This seems to be deprecated - why?

You should only catch specific errors you might expect (in this case IOError 
for failing to open the file) and let others pass (e.g. KeyboardInterrupt).

>    33           # Read the file & then close it
>    34           self.buff=self.fhand.read()
>    35           self.fhand.close()

Why not at once:

try:
    self.buff = file(self.fname).read()
except IOError:
    sys.exit('The file could not be opened')

>    54           len=find_len(self.buff[tbl_start:])

len() is a built-in function. I would not use it as variable name.

>    90       def __repr__(self):
>    91           """Display the contents of a font table"""
>    92           return (self.buff)

I would use __str__, that is what it is for.

>    97           fid_s=r'{\\f(\d+)'      # re to match font identifier
>    98   
>    99           fonts={}
>   100           fnum=0
>   101           i=0
>   102           for fontid in re.split(fid_s,self.buff):
>   103               if i % 2 == 1:
>   104                   fnum=fontid
>   105               else:
>   106                   fonts[fnum] = fontid
>   107               i+=1

See comment above.

>   164   def find_len(str):
>   165       """Find the length of the string up to a closing delimiter - 
>   166           eg the length of a font definition"""

How about this:

def find_len(str):
    L = 1       # I require the opening brace to be the first character
    try:
        while (str.count('{', 0, L) != str.count('}', 0, L)):
            L = str.index('}', L) + 1
    except ValueError:
        print "Premature EOF."
        sys.exit()
    return L


Sorry for snipping the rest of the code, but I know little about RTF, so I 
cannot comment on the program as a whole.

With kind regards, Rogier Steehouder