Code Review Stack Exchange is a question and answer site for peer programmer code reviews. Join them; it only takes a minute:

Sign up
Here's how it works:
  1. Anybody can ask a question
  2. Anybody can answer
  3. The best answers are voted up and rise to the top

I haven't done much Python in the past, so I was wondering if you could look over my code and make sure everything is up to snuff?

I know programs that do this exist already, but I am having a challenge with my friend on who can build a simple backup system for our college files.

I have run the code through Pylint and a PEP8 code checker, and fixed the errors it produced.

"""
Basic Python Backup
Nat Zimmermann
"""

# SETTINGS
TIME_FORMAT = '%d-%m-%Y-%H:%M:%S'
MAX_DIRS = 5

import shutil
from datetime import datetime
from os import walk


def get_dirs(dirs):
    """Get all immidiate directories in the current directory"""

    converted_dirs = []

    if len(dirs) > MAX_DIRS:
        test_dir = None

        for test_dir in dirs:
            try:
                app_dir = datetime.strptime(test_dir, TIME_FORMAT)
                converted_dirs.append(app_dir)

            except ValueError:
                print("Directory %s does not match date format, ignoring..."
                      % test_dir)

    return converted_dirs


def remove_dirs():
    """Remove the oldest directories"""

    dirs = walk('.').__next__()[1]
    converted_dirs = get_dirs(dirs)

    if converted_dirs:
        try:
            oldest_dir = datetime.strftime(min(converted_dirs), TIME_FORMAT)
            shutil.rmtree(oldest_dir)

            print("Removed old directory %s" % oldest_dir)

            remove_dirs()

        except IOError as error:
            print(error)


def copy_directory():
    """Copiy a given directory to a new directory with the current date"""

    old_directory = input("Directory to copy: ")
    new_directory = datetime.now().strftime(TIME_FORMAT)

    print("Copying...")

    try:
        shutil.copytree(old_directory, new_directory)
        print("Successfully copied files to directory %s" % new_directory)

    except IOError as error:
        print(error)
        copy_directory()

copy_directory()
remove_dirs()
share|improve this question
up vote 2 down vote accepted

Add following header for *nix users

#!/usr/bin/env python
# ~*~ codding: utf-8 ~*~

Not required variable definition

test_dir = None

Because of loop

for test_dir in dirs:

Use native generator insted

def get_dirs(dirs):
    """Get all immidiate directories in the current directory"""
    if not len(dirs) > MAX_DIRS:
        return []

    for test_dir in dirs:
        try:
            yield datetime.strptime(test_dir, TIME_FORMAT)
        except ValueError:
            print("Directory %s does not match date format, ignoring..."
                  % test_dir)

If you will use it like module, add

if __name__ == '__main__':
    copy_directory()
    remove_dirs()

IMHO: don't use direct input with input(), try parse command line instead using sys.argv or with help of some more powerful libraries.

share|improve this answer
1  
Thanks! I'm only using input() as a temporary measure, because I haven't tested it on my college system so I don't know what the full directory path of the files I want to backup is yet. Any reason you use ~*~ coding: utf-8 ~*~ instead of -*- coding: utf-8 -*-? – ntzm Sep 28 '14 at 18:51
    
Try to tap ~*~ and -*-. Don't you feel that? It's all because of typing speed. – outoftime Sep 28 '14 at 19:05
    
IMHO: and looks better. – outoftime Sep 28 '14 at 19:10
    
Easier on your keyboard, not on mine. (UK layout) – ntzm Sep 28 '14 at 21:15

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

Not the answer you're looking for? Browse other questions tagged or ask your own question.