Tracy R Reed wrote:

> Questions/problems/TODO's:
> 
> This is a fairly simple structured programming implementation. No OO.
> Should I be using some classes somewhere?

There doesn't seem to be any need for it.

> 
> The config file is just a module which I import which causes all of my
> configs to become globals. Globals are bad. Is there a better way or
> would just about anything else be overkill? A singleton config class or
> something? Overkill?

Instead of
from lollerskates_config import *

try
import lollerskates_config as config

or some other abbreviated name. Then instead of using e.g. 'macros' in 
your program use 'config.macros'. This has the advantage that it is 
clear where macros is defined and the config variables are not global to 
your module.

IMO singletons are greatly overused in general; in Python a module is a 
unique namespace.

> 
> I have several loops in this code for processing the logfiles. I once
> tried to convert these for loops to list comprehensions and totally
> confused myself and backed out the change (yeay svn!). Is there any
> benefit to list comprehensions in this case?

I don't see any loops that lend themselves to list comprehensions. A 
list comp is a shorthand way to build a new list from an existing list. 
You don't do that.

> 
> I would kinda like to play with unit tests. Not sure how I would
> construct unit tests for this. And again, perhaps overkill. But some
> people tell me you should write the unit tests before you even begin
> coding and code until the tests are satisfied. So even on a small
> project you would have tests. I run into a couple nasty bugs I created
> which caused the script to never return anything from the logfile (so
> you don't immediately realize something is broken) where I thought "It
> sure would be nice to have a test to catch that if it ever happens again."

Yep. Have you looked at the unit test module?

Many of your functions are unit-testable though it will be easier if you 
write them so they don't rely on external state (i.e. globals) (this is 
one reason globals are evil).

For example replace_tokens() could be tested, it would be easier if 
macros was a parameter instead of taken from the global state.

process_line() would be easier to test if it didn't rely on the global 
events list, but either took the list as a parameter or returned the 
line or None and let the caller deal with it.

Or maybe you do want a class that can hold the shared state...

One more note:
         if re.match("^$",line):
could just be
        if not line:

Overall it looks pretty clean and well-written.
Kent

_______________________________________________
Tutor maillist  -  Tutor@python.org
http://mail.python.org/mailman/listinfo/tutor

Reply via email to