6
\$\begingroup\$

A program that will count up the number of words in a text, as well as counting how many times a certain character shows up in the text. Only ignore spaces. This gave me a lot of trouble as I grappled with list comprehension syntax errors.

class TextAnalyzer:
    def __init__(self, txtFile):
        self.txtContent = open(txtFile)
        self.txtStream = self.txtContent.read()

    def __repr__(self):
        return self.txtStream

    def countChars(self):
        characters = {}
        for char in self.txtStream:
            if char != ' ' and char != '\n':
                if char.lower() not in characters:
                    characters[char.lower()] = 1
                else:
                    characters[char.lower()] += 1
        return [x for x in characters.items()]

    def countWords(self):
        totalWords = 0
        for b, a in enumerate(self.txtStream):
            if a != ' ' and a != '\n' and b != len(self.txtStream)-1 and (self.txtStream[b+1] == ' ' or self.txtStream[b+1] == '\n'):
                totalWords += 1
        return totalWords+1

analyze = TextAnalyzer('Darwin.txt')
totalChars = 0;
print(analyze)
for x, y in sorted(analyze.countChars()):
    totalChars += y
    print('{0}: {1}'.format(x, y))
print('Total Characters: ', totalChars)
print('Total Words: ', analyze.countWords())
\$\endgroup\$
7
\$\begingroup\$

Incorrect counting

For a text file that ends with a space or newline character, the countWords function counts 1 more word than there really are.

Before giving away the fix, I would point out a few things about this implementation:

totalWords = 0
for b, a in enumerate(self.txtStream):
    if a != ' ' and a != '\n' and b != len(self.txtStream)-1 and (self.txtStream[b+1] == ' ' or self.txtStream[b+1] == '\n'):
        totalWords += 1
return totalWords+1

The names b and a make this a bit hard to read. The value of a is a character, the value of b is the position (or index) of the character. Better names would have been c and i (or pos or index), and the code would be a bit easier to read.

Processing the text character by character like this is a bit complicated, and it will get even more complicated if you add the special treatment to fix the processing at the end of the content.

A simpler way would be to split the input, and use the length of the resulting list, for example:

return len(self.txtStream.split())

However, this is not exactly the same original code, and it has a drawback too. It's not the same, because the split() function of strings splits the input by any whitespace, including TAB, while your code splits strictly on SPACE and NEWLINE. If you really want to use the same logic and not split on TAB, then you would have to use the split function from the re package:

import re

# ...

return len(re.split('[ \n]', self.txtStream))

This preserves the behavior of your original code, and fixes the off-by-1 error, because just like the split function of strings, it ignores whitespace at the end of the content.

The splitting technique has an important drawback though: it consumes more memory, because it creates a list of words from the content, effectively doubling the memory use. Your solution is harder to implement correctly and to read, but it's more memory-efficient.

Always close resources

There are several issues here:

def __init__(self, txtFile):
    self.txtContent = open(txtFile)
    self.txtStream = self.txtContent.read()

def __repr__(self):
    return self.txtStream

The biggest issue is that a filehandle is not properly closed. When you open something, remember to close it!

Since it's so easy to forget closing filehandles, Python has a syntax to do this automatically for you:

with open(txtFile) as fh:
    self.content = fh.read()

Class attributes vs local variables

In the with example in the previous section, I renamed self.txtContent to fh, for two reasons:

  • It's a more appropriate name: txtContent implies some text content, which is not true and misleading. Likewise, I renamed txtStream to content, because that's what it really is. A "stream" is usually something that doesn't occupy much memory, doesn't contain the full content, but a data structure from which you can stream the content gradually without consuming much memory.

  • Even more importantly, self.txtContent is not used anywhere outside the __init__ function, so there's no need to make it an attribute of the class, it can be a local variable.

Avoid repeated operations

In this code, char.lower() is performed 3 times:

if char.lower() not in characters:
    characters[char.lower()] = 1
else:
    characters[char.lower()] += 1

Since all 3 times it will give the same result, it would be better to do this once, store the value in a variable and use that variable instead of repeating the call.

Avoid repeated operations -- the bigger picture

An even more important repeated operation could be performed by users of the TextAnalyzer class. Once you create an instance of this class, the text content it read from the file will normally not change, but calling countChars and countWords will recompute the same values over and over again.

If the class is not designed to change the value of the text content after construction, then it would be better to not compute the character and word counts only once.

Python conventions

There are common conventions for variable and function naming in Python, explained in PEP8. It's strongly recommended to follow that when coding in Python.

Simplifications

Instead of this:

if char != ' ' and char != '\n':

You can write like this:

if char not in ' \n':

Instead of this:

return [x for x in characters.items()]

You can write like this:

return characters.items()

Instead of this:

if char.lower() not in characters:
    characters[char.lower()] = 1
else:
    characters[char.lower()] += 1

You can write like this:

c = char.lower()
characters[c] = characters.get(c, 0) + 1
\$\endgroup\$
  • \$\begingroup\$ If I wanted to implement what you said about not doing the calculations again if the text hasn't changed, I would need a way to store the text file so that the program can compare between it and the next time it is used. My question is how would I do this? \$\endgroup\$ – Tony J Jan 4 '18 at 23:13
  • \$\begingroup\$ @TonyJ there are many ways to do that. One way is to create a new instance the next time you use it. \$\endgroup\$ – janos Jan 5 '18 at 7:08
3
\$\begingroup\$

For the counting of characters, you could use the collections.Counter class, which was introduced for exactly this. It even has a method most_common, which returns the items in order of descending occurrence. Your method would become:

from collections import Counter

class TextAnalyzer:
    ...
    def countChars(self):
        characters = Counter(char.lower()
                             for char in self.txtStream
                             if char not in ' \n')
        return characters.most_common()
\$\endgroup\$

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.