current community

your communities

more stack exchange communities

Stack Exchange
tour help
Take the 2-minute tour ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.
up vote 1 down vote favorite

I've just replaced my bash script with a Python script, and this is my first script where I used class.

Please review and give me suggestions.

#!/usr/bin/env python

# This Script will compress One day old file means yesterday files

from __future__ import print_function
import os
import sys
import time
import glob
import subprocess
from datetime import datetime

nday = 1        # set how many days old file
ftype = "*.log"     # set file type which should be compressed
lookinto = "/var/log/"  # path of files


class CheckFile(object):
    'Operation on file like get size,file time etc..'
    def __init__(self,fname):
        self.fname = fname
    def size(self):
        'print size in bytes MB'
        num = os.path.getsize(self.fname)
        for s in [ 'bytes','KB','MB','GB','TB' ]:
            if num < 1024.0:
                return "%3.1f %s" % (num,s) 
            num /= 1024.0
    def date(self):
        'print file date'
        self.unix_time = os.path.getmtime(self.fname)
        self.file_date = datetime.fromtimestamp(self.unix_time)
        return self.file_date.strftime('%d/%m/%y')
    def days_of_days_old(self):
        'print how much file is old from today'
        self.date()
        days = (datetime.today() - self.file_date).days
        return days 

def compresse_file(file):
    print("Compressing...")
    cmd = "gzip -v " + file
    os.system(cmd)


if __name__ == '__main__':
    os.chdir(lookinto)
    print('{0:10}{1:10}{2:10}'.format("Size","Date","File Name"))
    print('{0:10}{1:10}{2:10}'.format("-----","------","---------"))
    for file in glob.glob(ftype):
        f = CheckFile(file)
        if f.days_of_days_old() == nday:
            print('{0:10}{1:10}{2:10}'.format(f.size(),f.date(),f.fname))
            compresse_file(file)
            print("file status after compressesion")
            f = CheckFile(file + ".gz")
            print('{0:10}{1:10}{2:10}'.format(f.size(),f.date(),f.fname))
            print()
share|improve this question

1 Answer 1

up vote 4 down vote

First thing's first, instead of calling out to the gzip program with os.system, use the gzip module in Python. This will almost always be more portable: http://docs.python.org/2/library/gzip.html

Second of all, instead of using os.path.getsize and os.path.getmtime just use a single os.stat call: http://docs.python.org/2/library/os.html#os.stat You can call this once and cache the results for a single file.

Don't require calling self.date() in order to set the unix_time and file_date attributes. Instead make those into properties. For example you could write that like:

class CheckFile(object):
    'Operation on file like get size,file time etc..'

    def __init__(self, fname):
        self.fname = fname
        self._stat = None

    @property
    def stat(self):
        if self._stat is None:
            # Note: This will not update automatically if your file's stats change
            self._stat = os.stat(self.fname)
        return self._stat

    @property
    def unix_time(self):
        return self.stat.st_mtime

    @property
    def datetime(self):
        return datetime.fromtimestamp(self.unix_time)

    @property
    def age_in_days(self):
        return (datetime.today() - self.datetime).days

All that said, although I wouldn't discourage anyone from wanting to learn object-oriented programming, for this particular application writing a class is overkill :)

share|improve this answer
    
I haven't understand the @property, I read it's something call decorator, but could you please explain the same. –  Rahul Patil Dec 30 '13 at 22:41
    
I've read there is performance issue with gzip module that's why I use gzip command, see this post stackoverflow.com/questions/8302911/… –  Rahul Patil Dec 30 '13 at 22:56
    
There are some performance issues with the gzip module, especially where things like random access are concerned. But those are mostly corner cases. On most systems it should be using your system's zlib to do the compression and is more or less as fast for most cases. As for property there are tons of guides to that online. See for example stackoverflow.com/questions/6618002/… –  Iguananaut Dec 31 '13 at 1:20
    
thanks for your help.. :), I'm just concern that I've correctly define the class except property or not ? –  Rahul Patil Dec 31 '13 at 7:13
    
can you please check stackoverflow.com/questions/20856987/… –  Rahul Patil Dec 31 '13 at 12:01

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.