Counting the lines of a compressed file

Question

This is a Python script for counting the lines of a compressed file:

#!/usr/bin/env python3

import gzip, bz2, zipfile, os, sys

def openZip(zfile):
    f = zipfile.ZipFile(zfile)
    firstFile = f.filelist[0].filename
    if firstFile.endswith("/"):
        raise Exception("This zip file contains a directory.")
    return f.open(firstFile)

class compressedFile:
    fileOpenerDict = {".gz":gzip.open, ".bz2":bz2.open, ".zip":openZip}

    def __init__(self, pFilePath):
        self.filePath = os.path.abspath(pFilePath)
        self.fileStem, self.ext = os.path.splitext(self.filePath)
        self.ext = self.ext.lower()
        try:
            self.fileOpener = self.fileOpenerDict[self.ext]
        except KeyError:
            print("Only gz, zip, and bz2 files allowed.")
            exit(1)

    def countLines(self):
        nLines = 0
        with self.fileOpener(self.filePath) as fh:
            for line in fh:
                nLines = nLines + 1
        return nLines

if __name__ == "__main__":
    c = compressedFile(sys.argv[1])
    n = c.countLines()
    print("N lines: %d" % n)

Answer 1

Coding style

You should follow PEP8, the official coding style guide of Python. The most notable violations:

• Don't put all those imports on a single line, use multiple lines
• Use CamelCase for class names
• Use snake_case for function and variable names

Simplify and improve __init__

Don't create class member attributes that you don't need. For example self.fileStem is completely unused, and self.ext could be replaced with a local variable. Don't pollute your class with unnecessary elements.

Exception handling is best for dealing with anomalies. If the script is used with a file with an unsupported extension, that's not really an anomaly. It would be better to handle this with an if statement instead.

It's not a good idea to exit from the constructor of a class: the main method of the program should be in charge of shutting down or not. It would be better to define a custom exception, make the constructor raise it, and let the main method handle it.

The custom exception can be as simple as this:

class UnsupportedCompressionException(Exception):
    pass

The constructor with the above suggestions applied:

def __init__(self, filepath):
    ext = os.path.splitext(filepath)[1].lower()
    if ext not in self.fileOpenerDict:
        raise UnsupportedCompressionException("Only gz, zip, and bz2 files allowed.")
    self.filepath = filepath
    self.fileOpener = self.fileOpenerDict[ext]

The main method handling the exception:

if __name__ == "__main__":
    try:
        c = CompressedFile(sys.argv[1])
        n = c.count_lines()
        print("N lines: %d" % n)
    except UnsupportedCompressionException as e:
        print(e)
        exit(1)

Improve count_lines

It would be better to do the counting and return the result from inside the with. This is because in general it's best to limit all code to the smallest possible scope.

The line loop variable is unused. It's customary to use _ as the loop variable name in such case.

Instead of count = count + 1, it's more common and recommended to write count += 1.

As a result, the method becomes:

def count_lines(self):
    with self.fileOpener(self.filepath) as fh:
        count = 0
        for _ in fh:
            count += 1
        return count