On Mon, Nov 7, 2011 at 10:56 PM, steve <st...@lonetwin.net> wrote:
> Hey Shashidhar,
>
> Others may answer your question, I am not going to do that right away.
> I'll just offer a few suggestions to clean up your code. If you do that, I
> promise to answer your questions:
>
> a. Use os.path.join() to join path components
>
> b. You don't /have/ to read all the lines of the file into a list before
> processing it. The pythonic way would be:
>
> with open(filename) as input_file:
> for line in input_file.readline():
> ...do stuff ...
>
> c. You don't need to iterate over lists using numeric indexes. Even if you
> /need/ numeric index, use enumerate. So, instead of :
>
> for index in range(len(somelist)):
> ...do something with somelist[index] ...
> OR
> ...do something with index ...
>
> use something like:
>
> for element in somelist:
> ...do something with element...
> OR
> for index, element in enumerate(somelist):
> ...do something with index or element ....
>
>
> d. Don't chain string operations to just match a substring, use regular
> expressions. If you /have/ to chain string operations (for instance, to
> extract rather than match), chain them only once. So, instead of :
>
>
> if
> all_lines_in_file[line].split(**'->')[1].split(')')[0].split("**'")[1].strip()
> == "true":
> ...
> ...
>
> print "ZMP value %s" % all_lines_in_file[line].split(**
> '->')[1].split(')')[0].split("**'")[1].strip()
>
> ...and more such monstrosities, do either:
>
> stuff_that_interests_me = line.split('->')[1].split(')')**
> [0]("'")[1].strip()
>
> if stuff_that_interests_me == 'true':
> ....
>
> or just:
> matched = re.search("->\((.*)\)", line) # or some such, depending on the
> format
>
> if matched:
> stuff_that_interests_me = matched.group()
> ....
>
> e. If you need one of three options as values of a dict (which I assume
> will serve as a config dict), default the config keys to something. So, do:
>
> config = {'MyFirstKey' : 'ASK',
> 'MyNextKey' : 'ASK',
> ...
> }
>
> for line in all_lines:
> ...extract stuff_that_interests_me from line...
> if 'MyFirstKey' in line:
> if stuff_that_interests_me == 'true':
> config['MyFirstKey'] = 'ON'
> elif stuff_that_interests_me == 'false':
> config['MyFirstKey'] = 'OFF'
> ...
>
>
>
Excellent advise. Just adding 1 more point to this.
Kindly don't post your code here. Please use repositories
like pastebin for them and only post links here.
(I am thinking I should add these guidelines to the welcome
email...)
> HTH,
> cheers,
> - steve
>
>
--
--Anand
_______________________________________________
BangPypers mailing list
BangPypers@python.org
http://mail.python.org/mailman/listinfo/bangpypers
