I started to learn Python not long ago to automate certain tasks I work on. I want to optimize a script I wrote that takes a line in an Excel file and prints a pattern of text based on this line's content. Here's an example of output from my code:
mgmt_cli add access-rule layer "network" position bottom name "thirdline" source.1 "Value1front" source.2 "Value2front" source.3 "x3front" destination "Dest1front" service "HTTP" track.type "Log" action "Accept"
How would you optimize this code?
Link to the example file (just click Valider et Télécharger le Fichier) http://dl.free.fr/hFlGmywIN
Link to full code : https://pastebin.com/CR5SXHRQ
import xlrd
import re
loc = ("Dir\StackoverflowExample.xlsx")
wb = xlrd.open_workbook(loc)
sheet = wb.sheet_by_index(0)
sheet.cell_value(0, 0)
# Extracting number of rows
def CaseMulti(CellContent, CellName):
#takes a cell and a string as parameters, prints a pattern like Cell.X ="string", X starts with 0 and ends at the number of line in the Cell
case = str(CellContent)
x= re.split ("\s", case) #splits the cell contents in a list using regex where space is the separator for the list's elements
if name == CellContent: # deals with the cell with spaces between objects no clue how it work but it does
return "name " + '"{}"'.format(CellContent)
else:
s=0
string =""
anycells = ["any","Any", "ANY"] # tests if the soure, destination or service has any ==> returns nothing
if str(x[0]) in anycells :
return " "
else:
if len(x)==1: # case where te cell has one line (or a word with no spaces)
string= CellName + " " +'"{}"'.format(case)
else: # case where multilines/mutli spaces
while s<len (x) :
listelement = x[s]
string = string + CellName+'.'+str(s+1)+ ' ' +'"{}"'.format(listelement)
s=s+1
return string
for numberofrows in range (5):
print "Rule " + str (numberofrows)
name= sheet.cell_value(numberofrows,1)
source = sheet.cell_value(numberofrows,2)
destination = sheet.cell_value(numberofrows,3)
protocole = sheet.cell_value(numberofrows,4)
log = sheet.cell_value(numberofrows,5)
action = sheet.cell_value(numberofrows,6)
#firewall = sheet.cell_value(numberofrows,7)
try:
print ("mgmt_cli add access-rule layer \"network\" position bottom " + str(CaseMulti(name,"name")) + str(CaseMulti(source, " source")) + str(CaseMulti(destination, " destination")) + str(CaseMulti(protocole, " service")) + ' track.type "Log" ' + str(CaseMulti(action, " action")) )
except :
numberofrows +=1
2 Answers 2
STYLING
According to PEP-8, there are a few improvements to be made to your code:
- Imports: Your imports should go in alphabetical order
- Variable/Function Names: You should use
snake_case
instead ofcamelCase
- Parameter Names: This may just be how I view your code, but how your parameters are named look like your taking in objects. You should use lowercase variables for better readability.
- Spacing: You should use 4 spaces for each indent. For variable declaration and assignment, the
=
should be separated by a space on each side for readability, like so:s=0
tos = 0
. - Main Guard: You should wrap any regular code in a if 'name == 'main' guard, to make sure that the code is only being run when that file is the main file, and not being imported by anything.
STRING FORMATTING
All throughout your code, you put strings together using +
and .format
. Luckily there is an easier way, by putting an f
before the string like so: f"content here"
. This erases the need to have verbose +
in your strings, and you no longer have to cast int
to str
with this method, you can just write the code in the brackets {your code here}
.
Example:
return "name " + '"{}"'.format(cell_content)
to
return f"name \"{cell_content}\""
Here is the updated code:
import re
import xlrd
loc = ("Dir\StackoverflowExample.xlsx")
wb = xlrd.open_workbook(loc)
sheet = wb.sheet_by_index(0)
sheet.cell_value(0, 0)
# Extracting number of rows
def case_multi(cell_content, cell_name):
#takes a cell and a string as parameters, prints a pattern like Cell.X ="string", X starts with 0 and ends at the number of line in the Cell
case = str(cell_content)
x = re.split ("\s", case) #splits the cell contents in a list using regex where space is the separator for the list's elements
if name == cell_content: # deals with the cell with spaces between objects no clue how it work but it does
return f"name \"{cell_content}\""
else:
s = 0
string = ""
if str(x[0]).lower() == "any":
return " "
else:
if len(x) == 1: # case where the cell has one line (or a word with no spaces)
string = f"{cell_name} \"{case}\""
else: # case where multilines/mutli spaces
while s < len(x):
listelement = x[s]
string += f"{cell_name}.{str(s + 1)} \"{listelement}\""
s += 1
return string
def main():
for number_of_rows in range(5):
print "Rule " + str(number_of_rows)
name= sheet.cell_value(number_of_rows, 1)
source = sheet.cell_value(number_of_rows, 2)
destination = sheet.cell_value(number_of_rows, 3)
protocole = sheet.cell_value(number_of_rows, 4)
log = sheet.cell_value(number_of_rows, 5)
action = sheet.cell_value(number_of_rows, 6)
#firewall = sheet.cell_value(number_of_rows, 7)
try:
print ("mgmt_cli add access-rule layer \"network\" position bottom " + str(case_multi(name,"name")) + str(case_multi(source, " source")) + str(case_multi(destination, " destination")) + str(case_multi(protocole, " service")) + ' track.type "Log" ' + str(case_multi(action, " action")) )
except:
number_of_rows += 1
if __name__ == '__main__':
main()
-
\$\begingroup\$ To be clear, PEP-8 specifies that indentation should be 4 spaces per level, not a TAB character. This is an important convention to follow, since indentation matters in Python. \$\endgroup\$200_success– 200_success2019年07月05日 03:26:20 +00:00Commented Jul 5, 2019 at 3:26
-
\$\begingroup\$ @200_success Is a tab not already 4 spaces? I already had the clarification next to it I just assumed they were the same length \$\endgroup\$Ben A– Ben A2019年07月05日 03:27:21 +00:00Commented Jul 5, 2019 at 3:27
-
\$\begingroup\$ I would avoid any mention of "tab", since that just adds a possible misinterpretation: an ASCII character 9, and configuring your text editor or terminal to use 4-character-wide tabstops. \$\endgroup\$200_success– 200_success2019年07月05日 03:31:30 +00:00Commented Jul 5, 2019 at 3:31
It seems a main thing you need to learn about is f-strings
These are an update to the old format
method.
print "Rule " + str (numberofrows)
becomes
print(f"Rule str(numberofrows)")
and
"name " + '"{}"'.format(CellContent)
becomes
f"name {CellContent}"
NOTE Python 3 uses () for print statements
It is also worth using a PEP8 checker (such as black
- pip install black
) to make your code more readable
You could also change things like
s=s+1
to
s += 1
Rather than saying
anycells = ["any","Any", "ANY"]
if str(x[0]) in anycells
...
You could say
if str(x[0]).lower() == "any"
Are you sure this is your code? This looks very much like Python 2 code which hasn't been used for over 6 years. I am curious why you would write something in Python 2 still? It seems odd to learn Python 2 as a new beginner
-
\$\begingroup\$ thank you for your input. Yes this is my real code and it's in python 2, last time I coded was in Turbo Pascal that's why it looks very old \$\endgroup\$pythonoob– pythonoob2019年07月04日 22:50:00 +00:00Commented Jul 4, 2019 at 22:50
Explore related questions
See similar questions with these tags.