Simple interactive backup script

Question

How can I improve this?

GitHub

import os, platform, logging, logging.handlers, glob
from time import *
import tarfile
import zipfile, zlib
class backup:
 source_s = ''
 destination_d = ''
 def __init__(self):
 if strftime("%p") == 'AM':
 if strftime("%I") in ['12', '01', '02', '03', '04', '05', '06', '07', '08', '09', '10', '11']:
 time_d = 'Morning'
 else:
 pass 
 if strftime("%p") == 'PM':
 if strftime("%I") in ['12', '01', '02', '03']:
 time_d = 'Afternoon'
 elif strftime("%I") in ['04', '05', '06', '07', '08']:
 time_d = 'Evening'
 elif strftime("%I") in ['09', '10', '11']:
 time_d = 'Night'
 else:
 pass 
 print "Date:",strftime("%d %b %Y")
 print "Hi There, Today is ",strftime("%A")," And time is ", strftime("%I:%M:%S:%P"), ", So on this beautiful ",strftime("%A"),"", time_d," I welcome you to this Backup program."
 def source_destination(self):
 w_s = True
 w_ss = True
 while w_s:
 source = raw_input('Please Enter The Complete Path of Source Directory: ')
 if source == '':
 print "***You have not Specified Anything***"
 print "***Source Directory is required in Order to make backup***"
 continue
 elif os.path.exists(source) == False:
 print "***Source Path Does not Exists***"
 print "***Please check source path and Try Again***"
 continue
 else:
 print "***Specified source path is ACCEPTED***"
 w_s = False
 backup.source_s = source
 while w_ss:
 destination = raw_input('Please Enter Complete Path of Destination Directory:')
 destination = destination.replace(' ','_')
 if destination == '':
 print "***You have not Specified Anything***"
 print "***Destination Directory is required to store the backup files***"
 continue
 elif os.path.exists(destination) == False:
 print "***Destination Path Does not Exists***"
 decision = raw_input('Do you Want me to Create Destination For you (yes or no or just hit enter for yes):')
 if decision == 'yes':
 print "***Destination Path Will now be created***"
 os.mkdir(destination)
 print "***Directory has been created now***"
 print "***Program will now continue***"
 backup.destination_d = destination
 w_ss = False
 elif decision == 'no':
 print "***As You Wish, because Your wish is my Command***"
 print "***Program will now Terminate***"
 print "Bye !!!"
 exit()
 elif decision == '':
 print "***Destination Path Will now be created***"
 mk_d = 'mkdir {0}'.format(destination)
 os.system(mk_d)
 print "***Directory has been created now***"
 print "***Program will now continue***"
 backup.destination_d = destination
 w_ss = False
 elif decision not in ['yes','no','']:
 print "***What you think you are doing, you just have to choose yes or no, is it that HARD?***"
 print "***Try again now***"
 continue
 elif os.path.exists(destination) == True:
 if os.path.isdir(destination)== False:
 print "***Specified location is a file and not a directory, so try again and enter path of a directory***"
 continue
 else:
 print "***Specified destination path is ACCEPTED***"
 w_ss = False
 backup.destination_d = destination
 else:
 print "***Specified destination path is ACCEPTED***"
 w_ss = False
 backup.destination_d = destination
 def compress(self):
 source = backup.source_s
 destination = backup.destination_d
 w_sss = True
 f_name = raw_input('Please Enter The Desired name for output file(without extension):')
 if f_name == '':
 print "***You have not specified any name so DEFAULT will be used.(i.e 'untitled')***"
 f_name = 'untitled'
 else:
 pass
 while w_sss:
 c_method = raw_input('How you want your backup file to be compressed ?(tar, tar.gz, tar.bz2, or zip):') 
 if c_method == 'tar':
 ff_name = f_name.replace(' ', '_') + '.tar'
 w_sss = False
 elif c_method == 'tar.gz':
 ff_name = f_name.replace(' ', '_') + '.tar.gz'
 w_sss = False
 elif c_method == 'tar.bz2':
 ff_name = f_name.replace(' ', '_') + '.tar.bz2'
 w_sss = False
 elif c_method == 'zip':
 ff_name = f_name.replace(' ', '_') + '.zip'
 w_sss = False
 elif c_method == '':
 print "***You have not selected any method of compression***"
 print "***Please select atleast one method of compression***"
 continue
 else:
 print "***Sorry, The method you specified is not supported yet***"
 print "Please choose from the given options i.e tar, tar.gz, tar.bz2 or zip "
 continue
 suffix = ("/")
 if source.endswith(suffix) == True:
 pass 
 else:
 source = source + os.sep
 if destination.endswith(suffix) == True:
 pass 
 else:
 destination = destination + os.sep
 values = [source, destination, ff_name]
 if c_method == 'tar':
 print "***Compression can take sometime depending upon method you selected and the size of the source, so please be patient***"
 tar = tarfile.open(destination+ff_name, 'w')
 for item in os.listdir(source):
 print "Adding",item,"to archive"
 tar.add(os.path.join(source,item))
 tar.close()
 print "Operation successful"
 exit()
 elif c_method == 'tar.gz':
 print "***Compression can take sometime depending upon method you selected and the size of the source, so please be patient***"
 tar = tarfile.open(destination+ff_name, 'w:gz')
 for item in os.listdir(source):
 print "Adding",item,"to archive"
 tar.add(os.path.join(source,item))
 tar.close()
 print "Operation successful"
 exit()
 elif c_method == 'tar.bz2':
 print "***Compression can take sometime depending upon method you selected and the size of the source, so please be patient***"
 tar = tarfile.open(destination+ff_name, 'w:bz2')
 for item in os.listdir(source):
 print "Adding",item,"to archive"
 tar.add(os.path.join(source,item))
 tar.close()
 print "Operation successful"
 exit()
 else:
 print "***Compression can take sometime depending upon method you selected and the size of the source, so please be patient***"
 zipf = zipfile.ZipFile(destination+ff_name,"w", compression=zipfile.ZIP_DEFLATED)
 def recursive_zip(a, b):
 for item in os.listdir(b):
 if os.path.isfile(os.path.join(b, item)):
 print "Adding",item,"to archive"
 a.write(os.path.join(b, item))
 elif os.path.isdir(os.path.join(b, item)):
 recursive_zip(a, os.path.join(b, item))
 recursive_zip(zipf, source)
 zipf.close()
 print "Operation successful"
 exit()
try:
 p = backup()
 p.source_destination()
 p.compress()
except KeyboardInterrupt:
 print "Why are you leaving me "
 reason = raw_input("1. Your program is not good enough 2. I will be back (1 or 2):")
 if(reason == '1'):
 print "Thanks for using my program, I will try to Improve it, so till then, Good Bye !"
 exit();
 elif(reason == '2'):
 print "OK then, See you Soon"
 exit()
 else:
 print "Invalid Input !!!"
 exit()
except EOFError:
 print "Why are you leaving me "
 reason = raw_input("1. Your program is not good enough 2. I will be back (1 or 2):")
 if(reason == '1'):
 print "Thanks for using my program, I will try to Improve it, so till then, Good Bye !"
 exit();
 elif(reason == '2'):
 print "OK then, See you Soon"
 exit()
 else:
 print "Invalid Input !!!"
 exit()

Answer 1

import os, platform, logging, logging.handlers, glob
from time import *
import tarfile
import zipfile, zlib
class backup:

The python style guide recommends CamelCase for class names

 source_s = ''
 destination_d = ''

Why are these class attributes rather then instance attributes?

 def __init__(self):
 if strftime("%p") == 'AM':
 if strftime("%I") in ['12', '01', '02', '03', '04', '05', '06', '07', '08', '09', '10', '11']:
 time_d = 'Morning'

I recommend not using _d in your variable name, its not clear what it means. Name your variables with something meaningful.

 else:
 pass 
 if strftime("%p") == 'PM':
 if strftime("%I") in ['12', '01', '02', '03']:
 time_d = 'Afternoon'
 elif strftime("%I") in ['04', '05', '06', '07', '08']:
 time_d = 'Evening'
 elif strftime("%I") in ['09', '10', '11']:
 time_d = 'Night'
 else:
 pass 

Don't convert the time to a string, and then test it, it'll be easier if you get the time as numbers

current_time = time.localtime()
if 0 <= current_time.tm_hour < 12:
 time_d = 'Morning'

For a self-contained unit this size, I'd move it into a seperate function that returns time_d

 print "Date:",strftime("%d %b %Y")
 print "Hi There, Today is ",strftime("%A")," And time is ", strftime("%I:%M:%S:%P"), ", So on this beautiful ",strftime("%A"),"", time_d," I welcome you to this Backup program."
 def source_destination(self):
 w_s = True
 w_ss = True

Bad variables names, they are pretty inscrutable

 while w_s:
 source = raw_input('Please Enter The Complete Path of Source Directory: ')
 if source == '':
 print "***You have not Specified Anything***"
 print "***Source Directory is required in Order to make backup***"
 continue

This continue is pointless because the rest of the code is an else block, get rid of this.

 elif os.path.exists(source) == False:
 print "***Source Path Does not Exists***"
 print "***Please check source path and Try Again***"
 continue

Same here

 else:
 print "***Specified source path is ACCEPTED***"
 w_s = False

Use break, somebody may have told you not to use break, they were wrong. Break can be bad for readability, but setting boolean flags is worse.

 backup.source_s = source

Don't start data on your class, store it on self.

 while w_ss:

You set w_ss way above, don't do that. set variables close to their use

 destination = raw_input('Please Enter Complete Path of Destination Directory:')
 destination = destination.replace(' ','_')

Magically replacing parts of the path is suspicous

 if destination == '':
 print "***You have not Specified Anything***"
 print "***Destination Directory is required to store the backup files***"
 continue
 elif os.path.exists(destination) == False:
 print "***Destination Path Does not Exists***"
 decision = raw_input('Do you Want me to Create Destination For you (yes or no or just hit enter for yes):')
 if decision == 'yes':

use if decision == 'yes' or decision == '': to avoid duplicating this

 print "***Destination Path Will now be created***"
 os.mkdir(destination)
 print "***Directory has been created now***"
 print "***Program will now continue***"

Your script is overly printy. It says a lot more then it does.

 backup.destination_d = destination
 w_ss = False

as before, boolean flags are delayed gotos. Don't use them unless you must. Use break.

 elif decision == 'no':
 print "***As You Wish, because Your wish is my Command***"
 print "***Program will now Terminate***"
 print "Bye !!!"
 exit()

Doing this is generally not considered good form. There is nothing terribly wrong with it, but generally you should terminate your program by returning back to the main loop.

 elif decision == '':
 print "***Destination Path Will now be created***"
 mk_d = 'mkdir {0}'.format(destination)
 os.system(mk_d)

Why are you using system to make a directory? Use the os.mkdir function as you did above

 print "***Directory has been created now***"
 print "***Program will now continue***"
 backup.destination_d = destination
 w_ss = False
 elif decision not in ['yes','no','']:
 print "***What you think you are doing, you just have to choose yes or no, is it that HARD?***"
 print "***Try again now***"
 continue

Don't insult your users

 elif os.path.exists(destination) == True:

Don't use == True just use if os.path.exists(destination): if os.path.isdir(destination)== False:

Same with False. use if not os.path.isdir(destination)

 print "***Specified location is a file and not a directory, so try again and enter path of a directory***"
 continue
 else:
 print "***Specified destination path is ACCEPTED***"
 w_ss = False
 backup.destination_d = destination
 else:

Can this ever happen? You should either find yourself in the destination exists or destination does not exist category

 print "***Specified destination path is ACCEPTED***"
 w_ss = False
 backup.destination_d = destination

This function is too long. You should break it up. I'd want to make this function something like:

 def backup_destination(self):
 while True:
 destination = raw_input()
 if validate_destination(destination):
 self.destination = destination
 return 

See how short it is and how easy it is to see what its doing. All the details are tucked away in other functions.

 def compress(self):
 source = backup.source_s
 destination = backup.destination_d
 w_sss = True

You do realize you can reuse the same variable name across different functions don't you?

 f_name = raw_input('Please Enter The Desired name for output file(without extension):')
 if f_name == '':
 print "***You have not specified any name so DEFAULT will be used.(i.e 'untitled')***"
 f_name = 'untitled'
 else:
 pass

No need for an en empty else clause

 while w_sss:
 c_method = raw_input('How you want your backup file to be compressed ?(tar, tar.gz, tar.bz2, or zip):') 
 if c_method == 'tar':
 ff_name = f_name.replace(' ', '_') + '.tar'
 w_sss = False
 elif c_method == 'tar.gz':
 ff_name = f_name.replace(' ', '_') + '.tar.gz'
 w_sss = False
 elif c_method == 'tar.bz2':
 ff_name = f_name.replace(' ', '_') + '.tar.bz2'
 w_sss = False
 elif c_method == 'zip':
 ff_name = f_name.replace(' ', '_') + '.zip'
 w_sss = False

instead use

 if c_method in ('tar','tar.gz','tar.bz2','zip'):
 ff_name = f_name.replace(' ','_') + '.' + c_method
 break

That reduces duplication

 elif c_method == '':
 print "***You have not selected any method of compression***"
 print "***Please select atleast one method of compression***"
 continue
 else:
 print "***Sorry, The method you specified is not supported yet***"
 print "Please choose from the given options i.e tar, tar.gz, tar.bz2 or zip "
 continue
 suffix = ("/")

The parens do nothing

 if source.endswith(suffix) == True:
 pass 
 else:
 source = source + os.sep

Why are you using "/" above and os.sep here? Use os.sep everywhere

 if destination.endswith(suffix) == True:
 pass

Don't have empty if blocks, invert the logic of the if statement else: destination = destination + os.sep

You do the same thing to destination and source, write a function to do it

 values = [source, destination, ff_name]

Unused variable, delete it

 if c_method == 'tar':
 print "***Compression can take sometime depending upon method you selected and the size of the source, so please be patient***"

You have the same print in every if block, copy it outside of the if block

 tar = tarfile.open(destination+ff_name, 'w')

use os.path.join to combine pieces of a path. It will take care of the os.sep you were dealing with earlier

 for item in os.listdir(source):
 print "Adding",item,"to archive"
 tar.add(os.path.join(source,item))
 tar.close()
 print "Operation successful"
 exit()

again, exiting midscript is considered bad form. (in most cases)

 elif c_method == 'tar.gz':
 print "***Compression can take sometime depending upon method you selected and the size of the source, so please be patient***"
 tar = tarfile.open(destination+ff_name, 'w:gz')
 for item in os.listdir(source):
 print "Adding",item,"to archive"
 tar.add(os.path.join(source,item))
 tar.close()
 print "Operation successful"
 exit()

See how this is exactly the same as the first case, but with a different mode, write a function that takes the mode as a parameter

 elif c_method == 'tar.bz2':
 print "***Compression can take sometime depending upon method you selected and the size of the source, so please be patient***"
 tar = tarfile.open(destination+ff_name, 'w:bz2')
 for item in os.listdir(source):
 print "Adding",item,"to archive"
 tar.add(os.path.join(source,item))
 tar.close()
 print "Operation successful"
 exit()

And again!

 else:
 print "***Compression can take sometime depending upon method you selected and the size of the source, so please be patient***"
 zipf = zipfile.ZipFile(destination+ff_name,"w", compression=zipfile.ZIP_DEFLATED)
 def recursive_zip(a, b):
 for item in os.listdir(b):
 if os.path.isfile(os.path.join(b, item)):
 print "Adding",item,"to archive"
 a.write(os.path.join(b, item))
 elif os.path.isdir(os.path.join(b, item)):
 recursive_zip(a, os.path.join(b, item))
 recursive_zip(zipf, source)

Why are you supporting recursive for zip, but nobody else?

 zipf.close()
 print "Operation successful"
 exit()

Zip is different, but similiar to the others. I'd make it so they share code, but that's ab it trickier.

try:
 p = backup()

Don't use one letter variable names, unless the abbreviation is really common

 p.source_destination()
 p.compress()
except KeyboardInterrupt:
 print "Why are you leaving me "
 reason = raw_input("1. Your program is not good enough 2. I will be back (1 or 2):")
 if(reason == '1'):
 print "Thanks for using my program, I will try to Improve it, so till then, Good Bye !"
 exit();
 elif(reason == '2'):
 print "OK then, See you Soon"
 exit()
 else:
 print "Invalid Input !!!"
 exit()
except EOFError:
 print "Why are you leaving me "
 reason = raw_input("1. Your program is not good enough 2. I will be back (1 or 2):")
 if(reason == '1'):
 print "Thanks for using my program, I will try to Improve it, so till then, Good Bye !"
 exit();
 elif(reason == '2'):
 print "OK then, See you Soon"
 exit()
 else:
 print "Invalid Input !!!"
 exit()

use except (KeyboardInterrupt, EOFError): to catch multiple exceptions

Generally

1. You have a lot of useless continue statements that don't do anything
2. You use boolean flag variables, you should use break
3. Your variable names are cryptic
4. You duplicate a lot of logic. You should strive to have each piece of logic once
5. Your functions tend to be long and complicated, they should be broken up into smaller functions
6. Your compare boolean values with == True and == False DON'T!

• hsinxh: Thanks man, I am gonna rewrite this script by following your suggestions. Can you recommend me some good book/tutorial/paper so that I can be a good programmer?
• André Paramés: @Harbhag: I'd suggest Dive Into Python. Reading the Standard Library sources can be helpful as well.