Bad Code (that works) help me re-write!
Matthew Warren
Matthew.Warren at Digica.com
Wed Oct 11 05:20:03 EDT 2006
I have the following piece of code, taken from a bigger module, that
even as I was writing I _knew_ there were better ways of doing it, using
a parser or somesuch at least, but learning how wasn't as fun as coding
it... And yes alarm bells went off when I found myself typing eval(),
and I'm sure this is an 'unusual' use of for: else: . And I know it's
not very robust. As an ex cuse/planation, this is what happens when you
get an idea in your head and code it until it works, rather than doing
any kind of planning / designing first :)
I have a file that indicates the syntax of commands. User input is
checked against this file until it matches one of the lines in the file.
Then the indicated commands are run.
I think what I have ended up doing is writing a kind of a brute-force
parser for the file format I have defined. The trouble is, I'm sure
there are much more 'agile' and 'elegant' ways of achieveing this, and
also getting rid of eval() !
I'm posting here in the hope that it will criticised, laughed at, picked
apart and hence I shall learn a bit about python and how to do this
properly :)
The code follows, followed by the file defining the syntax of commands.
import re
def dbg(Msg,Fn):
try:
if (TRACE[Fn] or TRACE["ALL"]):
print 'DBG:'+Fn+': '+Msg
except KeyError:
pass
def processsubstitutions(RawCmd):
DBGBN='processsubstitutions'
infprotect=1
subhit=1
while subhit==1:
subhit=0
for sub in Substitutions:
SubCheck=RawCmd
RawCmd=re.sub('~'+sub,' '.join(Substitutions[sub]),RawCmd)
if SubCheck!=RawCmd:
#dbg('Made Substitution '+sub+' to get '+RawCmd,DBGBN)
subhit=1
infprotect=infprotect+1
if infprotect>100:
return "ERROR: Infinitely deep substitution levels
detected."
return RawCmd
#
#...processcommand is called with a string entered by the user.
#
def processcommand(Command):
DBGBN='processcommand'
dbg('Before Substitution: '+Command,DBGBN)
Command=processsubstitutions(Command) # if ! aliascmd then flow
is, RawCmd->subbed->executed
# is IS aliascmd
then flow is RawCmd->Subbed->aliashit->subbed->executed
dbg('After Substitution: '+Command,DBGBN)
AliasHit=0
CommandHit=0
SplitCmd=Command.split()
SplitLen=len(SplitCmd)
Cmd=SplitCmd[0]
InEtcRun=0
Error=0
CommandDefs=file(installroot+'FatControllerCommands.sav','r')
for Def in CommandDefs:
dbg("Scanning cdef ::"+Def,DBGBN)
if Def!='ENDCOMMANDDEFS':
DefTokens=Def.split()
ctr=0
for Token in DefTokens:
dbg("Doing token "+Token,DBGBN)
if re.search('input:',Token):
if SplitLen>ctr:
dbg("Is an input tag. ValExp=",DBGBN)
ValidateExpression=Token.replace('input:','').replace('<<',SplitCmd[ctr]
).replace('::SPACE::',' ')
dbg(ValidateExpression,DBGBN)
else:
Error=1
ErrorText='Error: Missing parameter.'
break
if not eval(ValidateExpression):##NOT SAFE NOT SAFE.
Need to come up with entirely new
##way to do all this
Error=1
ErrorText='Error: Parameter incorrect.'
break
elif re.search('create:',Token):
CreateExpression=Token.replace('create:','').replace('::SPACE::',' ')
elif Token!='+*':
if ctr>=SplitLen:
Error=1
ErrorText='Error: Bad command.'
break
if Token!=SplitCmd[ctr]:
Error=1
ErrorText='Error: Bad command.'
break
ctr+=1
CommandHit=1
else: #{EndOf for Token} all tokens found for else
eval(CreateExpression)
break
else: #{EndOf for Def} Check aliases
for AliasName in Aliases:
if Cmd==AliasName: #then, make cmdstring alias cmd string
and re-process
AliasCmd=' '.join(Aliases[AliasName])
AliasHit=1
dbg('Made alias hit to get '+AliasCmd,DBGBN)
break
else: #FOR loop else not an alias, so try execute as last
entity command
if not CommandHit:
global LastExecutedEntity
if LastExecutedEntity!='':
#global LastExecutedEntity
EntityManager.execute(LastExecutedEntity,SplitCmd[0:])
else:
print '\nError: Dont know which entity to use.\n'
else:
print '\nError: Bad command.\n'
if AliasHit==1:
AliasHit=0
CommandDefs.close()
processcommand(AliasCmd)
CommandDefs.close()
.....now the file. First I will explain what is in it;
The 'grammar'? Or file format I have defined is as follows. The aim is
to specify the syntax of commands that are then checked against user
input until a match is found, then eval() is used to call the function
given in the file. (bad I know)
There are sveral tokens in the file, this is what they mean
Input:(some condition, possibly containing '<<') - Whatever the user
has entered in this field replaces '<<' the resulting code must eval to
True.
Create:(function call using SplitCmd[]) - eval is used if
user input has matched the line to make the given function call,
SplitCmd is a list of the lines values.
+* indicates the user could enter any number of any parms , these are
handled by the code executed via the create: token.
....simple huh?
Here's the file 'FatControllerCommands.sav' (some lines are long, turn
off wrapping):
alias input:'<<'!='' +* create:definealias(SplitCmd[1],SplitCmd[2:])
define entity input:not(EntityManager.isEntity('<<')) input:'<<'!='' +*
create:EntityManager.define(SplitCmd[2],SplitCmd[3:])
def entity input:not(EntityManager.isEntity('<<')) input:'<<'!='' +*
create:EntityManager.define(SplitCmd[2],SplitCmd[3:])
delete entity input:EntityManager.isEntity('<<')
create:EntityManager.delete(SplitCmd[2])
del entity input:EntityManager.isEntity('<<')
create:EntityManager.delete(SplitCmd[2])
delete alias input:isalias('<<') create:delalias(SplitCmd[2])
del alias input:isalias('<<') create:delalias(SplitCmd[2])
delete substitution input:issubstitute('<<')
create:delsubstitution(SplitCmd[2])
delete sub input:issubstitute('<<') create:delsubstitution(SplitCmd[2])
del substitution input:issubstitute('<<')
create:delsubstitution(SplitCmd[2])
del sub input:issubstitute('<<') create:delsubstitution(SplitCmd[2])
execute input:EntityManager.isEntity('<<') input:'<<'!='' +*
create:EntityManager.execute(SplitCmd[1],SplitCmd[2:])
exec input:EntityManager.isEntity('<<') input:'<<'!='' +*
create:EntityManager.execute(SplitCmd[1],SplitCmd[2:])
x input:EntityManager.isEntity('<<') input:'<<'!='' +*
create:EntityManager.execute(SplitCmd[1],SplitCmd[2:])
exit create:sys.exit(0)
quit create:sys.exit(0)
help create:displayhelp()
load input:'<<'!='' create:load(SplitCmd[1])
save input:'<<'==('all') input:not(00)
create:save(SplitCmd[1],SplitCmd[2])
show entities create:EntityManager.show()
show aliases create:showaliases()
show substitutions create:showsubstitutions()
show subs create:showsubstitutions()
show daemons create:showdaemons()
substitute input:'<<'!=''
create:definesubstitution(SplitCmd[1],SplitCmd[2:])
sub input:'<<'!='' create:definesubstitution(SplitCmd[1],SplitCmd[2:])
trace input:'<<'!='' create:toggletrace(SplitCmd[1])
set input:'<<'!='' input:'<<'!='' input:'<<'!='' +*
create:SetOption(SplitCmd[1],SplitCmd[2],'::SPACE::'.join(SplitCmd[3:]))
show options create:displayopts()
define daemon input:'<<'!='' create:createdaemon(SplitCmd[2])
def daemon input:'<<'!='' create:createdaemon(SplitCmd[2])
define schedule input:IsDaemon('<<') input:'<<'!='' input:'<<'!=''
input:'<<'!=''
create:scheduledaemon(SplitCmd[2],SplitCmd[3],SplitCmd[4],SplitCmd[5])
define sched input:IsDaemon('<<') input:'<<'!='' input:'<<'!=''
input:'<<'!=''
create:scheduledaemon(SplitCmd[2],SplitCmd[3],SplitCmd[4],SplitCmd[5])
def schedule input:IsDaemon('<<') input:'<<'!='' input:'<<'!=''
input:'<<'!=''
create:scheduledaemon(SplitCmd[2],SplitCmd[3],SplitCmd[4],SplitCmd[5])
def sched input:IsDaemon('<<') input:'<<'!='' input:'<<'!=''
input:'<<'!=''
create:scheduledaemon(SplitCmd[2],SplitCmd[3],SplitCmd[4],SplitCmd[5])
define task input:IsDaemon('<<') input:'<<'!='' +*
create:adddaemontask(SplitCmd[2],SplitCmd[3],SplitCmd[4:])
define collector input:IsDaemon('<<') input:'<<'!='' input:'<<'!=''
input:'<<'!='' input:'<<'!='' input:'<<'!='' input:'<<'!=''
create:adddaemontaskcollector(SplitCmd[2],SplitCmd[3],SplitCmd[4],SplitC
md[5],SplitCmd[6],SplitCmd[7],SplitCmd[8])
subscribe entity input:IsDaemon('<<') input:'<<'!=''
input:EntityManager.isEntity('<<')
create:adddaemontaskentity(SplitCmd[2],SplitCmd[3],SplitCmd[4])
subs entity input:IsDaemon('<<') input:'<<'!=''
input:EntityManager.isEntity('<<')
create:adddaemontaskentity(SplitCmd[2],SplitCmd[3],SplitCmd[4])
def task input:IsDaemon('<<') input:'<<'!='' +*
create:adddaemontask(SplitCmd[2],SplitCmd[3],SplitCmd[4:])
def collector input:IsDaemon('<<') input:'<<'!='' input:'<<'!=''
input:'<<'!='' input:'<<'!='' input:'<<'!='' input:'<<'!=''
create:adddaemontaskcollector(SplitCmd[2],SplitCmd[3],SplitCmd[4],SplitC
md[5],SplitCmd[6],SplitCmd[7],SplitCmd[8])
def alert input:IsDaemon('<<') input:'<<'!='' input:'<<'!=''
input:'<<'!='' input:'<<'!='' input:'<<'!=''
create:adddaemontaskcollectoralert(SplitCmd[2],SplitCmd[3],SplitCmd[4],S
plitCmd[5],SplitCmd[6],'::SPACE::'.join(SplitCmd[7:]))
define alert input:IsDaemon('<<') input:'<<'!='' input:'<<'!=''
input:'<<'!='' input:'<<'!='' input:'<<'!=''
create:adddaemontaskcollectoralert(SplitCmd[2],SplitCmd[3],SplitCmd[4],S
plitCmd[5],SplitCmd[6],'::SPACE::'.join(SplitCmd[7:]))
delete daemon input:IsDaemon('<<') create:removedaemon(SplitCmd[2])
del daemon input:IsDaemon('<<') create:removedaemon(SplitCmd[2])
delete task input:IsDaemon('<<') input:'<<'!=''
create:removedaemontask(SplitCmd[2],SplitCmd[3])
del task input:IsDaemon('<<') input:'<<'!=''
create:removedaemontask(SplitCmd[2],SplitCmd[3])
update task input:IsDaemon('<<') input:'<<'!='' input:'<<'!=''
create:updatedaemontask(SplitCmd[2],SplitCmd[3],SplitCmd[4:])
upd task input:IsDaemon('<<') input:'<<'!='' input:'<<'!=''
create:updatedaemontask(SplitCmd[2],SplitCmd[3],SplitCmd[4:])
delete collector input:IsDaemon('<<') input:'<<'!='' input:'<<'!=''
create:removedaemontaskcollector(SplitCmd[2],SplitCmd[3],SplitCmd[4])
del collector input:IsDaemon('<<') input:'<<'!='' input:'<<'!=''
create:removedaemontaskcollector(SplitCmd[2],SplitCmd[3],SplitCmd[4])
unsubscribe entity input:IsDaemon('<<') input:'<<'!='' input:'<<'!=''
create:removedaemontaskentity(SplitCmd[2],SplitCmd[3],SplitCmd[4])
unsub entity input:IsDaemon('<<') input:'<<'!='' input:'<<'!=''
create:removedaemontaskentity(SplitCmd[2],SplitCmd[3],SplitCmd[4])
activate daemon input:IsDaemon('<<') create:makedaemonlive(SplitCmd[2])
act daemon input:IsDaemon('<<') create:makedaemonlive(SplitCmd[2])
show active daemons create:showactivedaemons()
deactivate daemon input:IsDaemon('<<') create:killdaemon(SplitCmd[2])
deac daemon input:IsDaemon('<<') create:killdaemon(SplitCmd[2])
alerts create:showalertqueue()
handle input:'<<'!='' input:'<<'!=''
create:handlealertrange(int(SplitCmd[1]),int(SplitCmd[2]))
addline input:'<<'!='' +*
create:appendtoscript(SplitCmd[1],SplitCmd[2:])
insline input:'<<'!='' input:'<<'!='' +*
create:inserttoscript(SplitCmd[1],SplitCmd[2],SplitCmd[3:])
delline input:'<<'!='' input:'<<'!=''
create:delfromscript(SplitCmd[1],SplitCmd[2])
run input:isScript('<<') +* create:runscript(SplitCmd[1],SplitCmd[2:])
delete script input:'<<'!='' create:delscript(SplitCmd[2])
del script input:'<<'!='' create:delscript(SplitCmd[2])
show script input:'<<'!='' create:showscripts(SplitCmd[2])
show scripts create:showscripts('all')
message input:'<<'!='' +* create:message(SplitCmd[1:])
msg input:'<<'!='' +* create:message(SplitCmd[1:])
ENDDCOMMANDDEFS
Thankyou for your time pythoners, any suggestions / comments are much
appreciated.
Matt.
This email is confidential and may be privileged. If you are not the intended recipient please notify the sender immediately and delete the email from your computer.
You should not copy the email, use it for any purpose or disclose its contents to any other person.
Please note that any views or opinions presented in this email may be personal to the author and do not necessarily represent the views or opinions of Digica.
It is the responsibility of the recipient to check this email for the presence of viruses. Digica accepts no liability for any damage caused by any virus transmitted by this email.
UK: Phoenix House, Colliers Way, Nottingham, NG8 6AT UK
Reception Tel: + 44 (0) 115 977 1177
Support Centre: 0845 607 7070
Fax: + 44 (0) 115 977 7000
http://www.digica.com
SOUTH AFRICA: Building 3, Parc du Cap, Mispel Road, Bellville, 7535, South Africa
Tel: + 27 (0) 21 957 4900
Fax: + 27 (0) 21 948 3135
http://www.digica.com
More information about the Python-list
mailing list