patternpythonMinor
Minecraft auto-backup utility
Viewed 0 times
autominecraftutilitybackup
Problem
I live in a country where an electricity blackout happens after every hour, which corrupts the Minecraft save files, so I came up with this auto backup utility which backs up the %appData%.minecraft\saves folder constantly after a specified time.
It works fine but I'm still not satisfied with it. Tell me what more I can do to improve/enhance it.
```
import time
import os
import sys
#class for parsing settings.ini
class parseIni(object):
def __init__(self, fileStream):
self.lines = fileStream.readlines()
self.options = []
for line in self.lines:
if "=" not in line:
continue
else:
self.options.append(tuple(line.replace("\n", "").split("=")))
def getValueOf(self, option):
for op, value in self.options:
if op==option:
return value
#Tries to open the settings file, if unsuccessful then exit program
settingsFileStream = None
try:
settingsFileStream = open("settings.ini")
except IOError:
print("ERROR: SETTINGS FILE NOT FOUND.", file=sys.stderr)
raw_input("Press to exit....")
sys.exit(0)
#Initialize parser
settings = parseIni(settingsFileStream)
#Precondition: settings file was opened successfully
#Assigning values to required variables
timeDelayInMins = eval(settings.getValueOf("timeDelay")) #For user information
timeDelay = timeDelayInMins * 60 #Converting to seconds, for time.sleep() method
sourcePath = settings.getValueOf("sourcePath")
destinationPath = settings.getValueOf("destinationPath")
#Runs the game
os.system("Minecraft.exe")
#Prints out Welcome message
print()
print("|--------------------------------------------------------|")
print("|WELCOME TO MINECRAFT AUTO BACKUP UTILITY! |")
print("|--------------------------------------------------------|")
print("|REFER TO THE settings.ini FILE TO EDIT SETTINGS FOR THIS|")
print("|PROGRAM. e.g. TIME DELAY FOR AUTO BACKUP, PATHS etc |")
print("|---------
It works fine but I'm still not satisfied with it. Tell me what more I can do to improve/enhance it.
```
import time
import os
import sys
#class for parsing settings.ini
class parseIni(object):
def __init__(self, fileStream):
self.lines = fileStream.readlines()
self.options = []
for line in self.lines:
if "=" not in line:
continue
else:
self.options.append(tuple(line.replace("\n", "").split("=")))
def getValueOf(self, option):
for op, value in self.options:
if op==option:
return value
#Tries to open the settings file, if unsuccessful then exit program
settingsFileStream = None
try:
settingsFileStream = open("settings.ini")
except IOError:
print("ERROR: SETTINGS FILE NOT FOUND.", file=sys.stderr)
raw_input("Press to exit....")
sys.exit(0)
#Initialize parser
settings = parseIni(settingsFileStream)
#Precondition: settings file was opened successfully
#Assigning values to required variables
timeDelayInMins = eval(settings.getValueOf("timeDelay")) #For user information
timeDelay = timeDelayInMins * 60 #Converting to seconds, for time.sleep() method
sourcePath = settings.getValueOf("sourcePath")
destinationPath = settings.getValueOf("destinationPath")
#Runs the game
os.system("Minecraft.exe")
#Prints out Welcome message
print()
print("|--------------------------------------------------------|")
print("|WELCOME TO MINECRAFT AUTO BACKUP UTILITY! |")
print("|--------------------------------------------------------|")
print("|REFER TO THE settings.ini FILE TO EDIT SETTINGS FOR THIS|")
print("|PROGRAM. e.g. TIME DELAY FOR AUTO BACKUP, PATHS etc |")
print("|---------
Solution
import time
import os
import sys
#class for parsing settings.ini
class parseIni(object):Python style guide recommends names like
ParseInidef __init__(self, fileStream):Python style guide recommends argument like
file_stream.self.lines = fileStream.readlines()You don't use the lines again, so why are you storing them on the object. There is not a whole lot of point in calling deadlines, because you can actually just pass the file object in the for loop for the same effect
self.options = []
for line in self.lines:
if "=" not in line:
continueI suggest almost always avoiding continue. As it is continue does nothing, replacing with pass will have the same effect. The whole if can be simplified by inverting the condition
else:
self.options.append(tuple(line.replace("\n", "").split("=")))For clearing out excess whitespace, I recommend using
line.strip(). I'd also suggest having options be a dictionary. Then you could dokey, value = line.strip().split('=')
self.options[key] = valueIt'll make lookup more efficient and is a bit simpler
def getValueOf(self, option):Awkward and longish name. I'd have this be
__getitem__, and then you can access it with the [] operatorfor op, value in self.options:
if op==option:
return valueThis is the for-if anti-pattern. Whenever you find yourself writing an if which should only be true for one instance of the surrounding loop, you should look for a better solution.
Also, what happens if the option is missing? You should probably raise an exception.
#Tries to open the settings file, if unsuccessful then exit program
settingsFileStream = NoneNo point in doing this.
try:
settingsFileStream = open("settings.ini")
except IOError:
print("ERROR: SETTINGS FILE NOT FOUND.", file=sys.stderr)
raw_input("Press to exit....")
sys.exit(0)You should probably print the exception itself. As it is you are throwing away potentially useful error information.
#Initialize parser
settings = parseIni(settingsFileStream)
#Precondition: settings file was opened successfully
#Assigning values to required variables
timeDelayInMins = eval(settings.getValueOf("timeDelay")) #For user informationUsing eval is a bad idea. It'll try to evaluate any valid python you pass to it which could allow an external file to cause your application to do bad things. You also can't be sure what kind of data you got back. Instead pass the incoming string to
int or float to safely convert. timeDelay = timeDelayInMins * 60 #Converting to seconds, for time.sleep() method
sourcePath = settings.getValueOf("sourcePath")
destinationPath = settings.getValueOf("destinationPath")
#Runs the game
os.system("Minecraft.exe")
#Prints out Welcome message
print()
print("|--------------------------------------------------------|")
print("|WELCOME TO MINECRAFT AUTO BACKUP UTILITY! |")
print("|--------------------------------------------------------|")
print("|REFER TO THE settings.ini FILE TO EDIT SETTINGS FOR THIS|")
print("|PROGRAM. e.g. TIME DELAY FOR AUTO BACKUP, PATHS etc |")
print("|--------------------------------------------------------|")
print("|TIME DELAY FOR AUTO BACKUP IS {:0.2f} MIN(S) |".format(timeDelayInMins))
print("|--------------------------------------------------------|")
print()
#Variable to keep tract of backups done per session
backupsDone = 0
#mainLoop runs forever, backups "saves" folder every after ever timeDelay mins
while True:
time.sleep(timeDelay)
os.system('robocopy "{}" "{}" /e'.format(sourcePath, destinationPath))Python has function for copying files. Consider using that instead of calling to another program.
backupsDone += 1
print
print
print("|---------------------------------|")
print("|{} BACKUP(S) DONE THIS SESSION. |".format(backupsDone))
print("|---------------------------------|")It would also be a good idea to move all this logic into a main() function.
Code Snippets
import time
import os
import sys
#class for parsing settings.ini
class parseIni(object):def __init__(self, fileStream):self.lines = fileStream.readlines()self.options = []
for line in self.lines:
if "=" not in line:
continueelse:
self.options.append(tuple(line.replace("\n", "").split("=")))Context
StackExchange Code Review Q#13358, answer score: 4
Revisions (0)
No revisions yet.