1
\$\begingroup\$

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()
\$\endgroup\$
4
\$\begingroup\$

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 :)

\$\endgroup\$
  • \$\begingroup\$ I haven't understand the @property, I read it's something call decorator, but could you please explain the same. \$\endgroup\$ – Rahul Patil Dec 30 '13 at 22:41
  • \$\begingroup\$ I've read there is performance issue with gzip module that's why I use gzip command, see this post stackoverflow.com/questions/8302911/… \$\endgroup\$ – Rahul Patil Dec 30 '13 at 22:56
  • \$\begingroup\$ 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/… \$\endgroup\$ – Iguananaut Dec 31 '13 at 1:20
  • \$\begingroup\$ thanks for your help.. :), I'm just concern that I've correctly define the class except property or not ? \$\endgroup\$ – Rahul Patil Dec 31 '13 at 7:13
  • \$\begingroup\$ can you please check stackoverflow.com/questions/20856987/… \$\endgroup\$ – Rahul Patil Dec 31 '13 at 12:01

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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