Is there a better way of doing this?

[Steven D'Aprano]

On Sat, 28 May 2005 13:24:19 +0000, Michael wrote:

> Hi,
> I'm fairly new at Python, and have the following code that works but isn't
> very concise, is there a better way of writing it?? It seems much more
> lengthy than python code i have read..... :-)
> (takes a C++ block and extracts the namespaces from it)


Does it work? If Yes, then "if it works, don't fix it".

Is it fast enough? If Yes, then "don't optimise prematurely".

Does it do everything you need it to do? If Yes, then "know when to stop
programming".

Is it written in good Python style? If Yes, then "leave it alone".

The last one is the most difficult, because nobody agrees what good
Python style is. A lot of it depends on who is going to be reading your
code. Personally, I worry about folks who try to turn every piece of code
into a one-liner. If you want to write perl, write perl. Just because you
can write something as a nested list comprehension doesn't mean you should.

Having said that, I'll make some stylistic suggestions:
 
> def ExtractNamespaces(data):
>  print("Extracting Namespaces")

print is a statement, not a function. The brackets are syntactically
correct, but pointless. Remove them.

>  p = re.compile( 'namespace (?P<name>[\w]*)[\n\t ]*{')
> 
>  subNamespaces = []
> 
>  newNS = p.search(data)
>  while( newNS ):

Guido (our Benevolent Dictator For Life and creator of Python) hates
seeing whitespace next to parentheses. I agree with him. while(newNS)
good, while( newNS ) bad.

See http://www.python.org/doc/essays/styleguide.html for Guido's
suggestions.

>   print "\t" + newNS.group("name")
> 
>   OPCount = 1
>   Offset = newNS.end()
>   while(OPCount > 0):
>    if( data[Offset] == "}" ):

See above comment about whitespace inside brackets.

More importantly, you don't need the brackets at all. You can write:

if data[Offset] == "}":

which is less visually confusing. The only time I use brackets in an if
statement is to clarify complex Boolean expressions, eg

if myFlag and ((somevalue == 3) or (num_fibberts != 1)):

Otherwise, drop the brackets.

>     OPCount = OPCount -1;

You aren't writing Pascal or C now, so you don't need the semi-colon. The
semi-colon isn't wrong, as such, since Python allows you to put multiple
statements on a single line separated with semi-colons, but it isn't
recommended and will give away the fact that you are (1) a newbie and (2)
not really comfortable with Python.

In more recent versions of Python, you can also write that as OPCount -= 1

>    elif( data[Offset] == "{" ):
>     OPCount = OPCount + 1;
>    Offset = Offset+1;

Again, drop the brackets from the elif statement and the semi-colons. Not
the colons, they are required!

>   #Extract Data:

More comments! Comments are good. There are those who claim that you
should write more comments than code. I don't quite agree with that, but
more detailed comments would not go astray.

>   newNSData = data[newNS.end():Offset-1] 
>   data = data[0:newNS.start()] + data[Offset:]
>   newNamespace = [newNS.group("name"), newNSData];
>   subNamespaces.append(newNamespace)

By now you should be sick of me telling you not to use semi-colons.

>   #Perform NewSearch
>   newNS = p.search(data)
>  return [subNamespaces,data]

A single space after commas helps make the phrase more readable.

Other than those stylistic comments, and the need for more comments, it
looks good to me.


-- 
Steven