Bad Code (that works) help me re-write!

[Matthew Warren]

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

Inputsome condition, possibly containing '<<') - Whatever the user
has entered in this field replaces '<<' the resulting code must eval to
True.

Createfunction 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

 

[Giles Brown]

I have the following piece of code,

<snip>
No doubt you will get some kind soul to suggest some things, but if you
want really good
answers I think you need explain why you want this command file (as
opposed to using
say a python script itself). Are you attempting to create a simpler
syntax than Python?
A restricted set of operations?

Giles

 

[MatthewWarren]

oop. posted with wrong account, sorry for attatched disclamers in other
posts.

 

[MatthewWarren]

-----Original Message-----
From:
[email protected]
[mailto[email protected]
rg] On Behalf Of Giles Brown
Sent: 11 October 2006 12:38
To: (e-mail address removed)
Subject: Re: Bad Code (that works) help me re-write!

I have the following piece of code,

<snip>
No doubt you will get some kind soul to suggest some things,
but if you
want really good
answers I think you need explain why you want this command file (as
opposed to using
say a python script itself). Are you attempting to create a simpler
syntax than Python?
A restricted set of operations?

The code is from a larger project called the FatController.

FatController is currently a cli based utility for administering &
monitoring devices & applications in an enterprise environment.

Users enter commands at the prompt do things such as, define a managed
entity, execute instructions against that entity, etc

The processcommand() function and the command file are my attempt at
implementing the 'commands' that are available to the user. Rather than
hard-code all of the syntax and commands inside the module, I wanted an
external file to maintain the definitions of the commands available to
the user. The idea is that users will be able to implement their own
'entity' modules, which FatController will import and then use to manage
devices & applications the given entty applies to. So, at the time of
writing the code thought It would be a good idea to have the command
definitions file external to the code, so and-users who write their own
modules can also extend the cammands available to Fatcontroller for
using those modules, if required.



Matt.

More info. An entity 'module' is something that creates connections to
an app/device/machine using whatever protocol, sends native commands to
that device/app/machine, and returns the result. For example, I have
written an entity of type 'TELNET' that manages anything that you can
use telnet to connect to (hence prev. post on using twisted rather than
telnetlib) . The user could enter something like the following at the
Fatcontroller prompt;

FC:> define entity TELNET unixbox1 192.168.4.5 23 mylogin mypass

the user could then write something like;

FC:> execute unixbox1 ps -ef | grep -c crit_process

the processcommand() function and the command definition file are used
to parse/analyse the user input and pass the relevant parms to the
entity module that then defines the entity for later use/managment, or
pass the given entity command to the entity for execution, and return
of the output. Fatcontroller provides other functionality for working
with entities / groups of entities, scheduling commands to be run
against entites / groups of entities and saving / extracting / alerting
against the output.


....but really, all I'm after is a learning experience, (this whole
project is what I decided to do to learn python) and I know
processcommand() can be written in a more elegant way and that eval()
shouldnt be there, but I have no formal acquaintance with things like
parsers and the subtleties of pythons approach etc.. So I'm hoping
through deconstructing and rebulding the code I can increase my
knowledge of python and the pythonic way to do things.

Aaanyhoo,

Ta

Matt.



Matt.