5
\$\begingroup\$

I'm new to Python and want to develop good habits at the beginning. This class takes a string, and then counts the number of vowels and consonants in the word(s). Any advice on how to improve?

class CntVowels:

    def __init__(self,string):
        self.string = string
        self.vowels = [ "a", "e", "i", "o", "u"]

    def vowel_count(self):
        strings = self.string.lower()
        vowels = self.vowels

        counter = 0
        for letter in strings:
            for vowel in vowels:
                if letter == vowel:
                    counter += 1
        print("There are {} vowels in this string.".format(counter))

    def constant_count(self):
        strings = self.string.lower()
        vowels = self.vowels
        length = len(strings)
        counter = 0
        for letter in self.string:
            for vowel in vowels:
                if letter == vowel:
                    counter += 1

        results = length - counter
        print("There are {} constants in this string.".format(results))



a = input("enter a string")

CntVowels(a).vowel_count()

CntVowels(a).constant_count()

\$\endgroup\$
  • \$\begingroup\$ Note: You mean “consonants”, not “constants”. I’ve edited the title & question text, but I’ve left the code unchanged. \$\endgroup\$ – AJNeufeld Aug 28 at 0:34
4
\$\begingroup\$

Welcome to Python, and welcome to Code Review.

Classes

An instance of a class, where only one method is ever called, is not a good class. Instead, use functions.

a = input("Enter a string: ")
vowel_count(a)
constant_count(a)

Class Constants & Members

If you do create a class, and that class has some global constant data, store that on the class itself, not on the instances of the class.

Constants should have UPPERCASE_NAMES, to distinguish them from non-constant data. Moreover, if the data is constant, where possible use non-mutable objects, such as tuple (eg, (..., ..., ...)) over list (eg, [..., ..., ...]).

Non-public members of a class should begin with a single underscore.

class CntVowels:

    VOWELS = ("a", "e", "i", "o", "u")

    def __init__(self, string):
        self._string = string

    # ...

Eschew .lower()

When attempting to compare strings (or characters) case-insensitively, use .casefold() instead of .lower(). This properly handles many additional oddities, such as the German ß which is equivalent to ss.

x in y

Testing for existence of an item in a container is easily done with the in operator. For example, this code:

for vowel in vowels:
    if letter == vowel:
        counter += 1

can be re-written more efficiently as:

if letter in vowels:
    count += 1

A String is a Container

It could even be re-written as:

if letter in "aeiou":
    count += 1

since a string is simply a container of letters.

Non-Vowels are not necessarily Consonants

Many characters which are not vowels are also not consonants, such as 4, &, and . to name a few.

Avoid Name Shadowing

string is not a good variable name, because a module named string exists, and can be imported, and doing so results in the meaning of string changing depending on what is in scope.

It is even reasonable that you’d import string in this program, to get convenient access to all of the string.ascii_lowercase letters.

\$\endgroup\$
  • \$\begingroup\$ Would it be more conventional to count ß as one consonant or two? \$\endgroup\$ – 200_success Aug 28 at 5:49
  • \$\begingroup\$ @200_success from my limited understanding of German it is one letter. However, Wikipedia says "The grapheme has an intermediate position between letter and ligature." en.wikipedia.org/wiki/%C3%9F \$\endgroup\$ – Brian J Aug 28 at 15:59
  • \$\begingroup\$ Rocket Languages states it (``) is a consonant, which means .foldcase, while great for equivalents tests, isn’t so good for consonant counting. \$\endgroup\$ – AJNeufeld Aug 28 at 18:31
3
\$\begingroup\$
if __name__ == '__main__':

use this guard to test your functions at the end of the script in the following way:

if __name__ == '__main__':
    a = input("enter a string")
    CntVowels(a).vowel_count()
    CntVowels(a).constant_count()

Functions do not print: your functions should return values and these values should be indicated in the docstrings example:

def vowel_count(self):
    """Return vowel count."""
    return len([letter for letter in self.string if letter in self.vowels])

def consonant_count(self):
    """Return consonant count."""
    return len([letter for letter in self.string if letter not in self.vowels])

There is no need to use a class, for such simple tasks a function is sufficient and the whole code might look like this:

def get_total_count(word):
    """Return vowel and consonant count in a word."""
    vowels = [letter for letter in word if letter in 'aeiou']
    consonants = [letter for letter in word if letter not in 'aeiou']
    return len(vowels), len(consonants)


if __name__ == '__main__':
    word = input('Enter a word: ').lower()
    while not word.isalpha():
        print('Invalid word! Please enter letters only.')
        word = input('Enter a word: ').lower()
    vowel_count, consonant_count = get_total_count(word)
    print(f"There are ({vowel_count}) vowels and ({consonant_count} consonants) in '{word}'.")
\$\endgroup\$
  • 2
    \$\begingroup\$ No. Do not use len( [ ... ] ). This is unnecessarily creating a new object (a list), simply to determine the length of said object. sum(1 for letter in self.string if letter in self.vowels) while slightly cryptic is a better alternative. More efficient (and even more cryptic) would be sum(letter in self.vowels for letter in self.string). \$\endgroup\$ – AJNeufeld Aug 28 at 0:29
  • \$\begingroup\$ Thank you for pointing this out, a lot of times I feel like I'm over relying on lists and memory to do the work. \$\endgroup\$ – user203258 Aug 28 at 0:30
  • \$\begingroup\$ @AJNeufeld The second one is actually less efficient (because it sums also the zeros/False). \$\endgroup\$ – Graipher Aug 28 at 8:35
2
\$\begingroup\$

If your class counts not only vowels but also consonants, why call it CntVowels? CntSounds or CntLetters might be more suitable.

\$\endgroup\$
1
\$\begingroup\$

If you are starting out in Python, you should have a look at it's standard library. It has many helpful modules. One of them is the [collections][2] module, which contains a [Counter][2] object that, well, counts stuff. You could use it here to count the occurrence of each letter in the given string and calculate the number of consonants and vowels from that. In addition, have a look at the [string][2] module that contains some useful constants.

As said in the other answers, a simple function is enough for this task, no need to make it a class.

import string
from collections import Counter

VOWELS = set("aeiou")
CONSONANTS = set(string.ascii_lowercase) - VOWELS

def count_vowels_consonants(s):
    counts = Counter(s.casefold())
    vowels = sum(counts[c] for c in VOWELS)
    consonants = sum(counts[c] for c in CONSONANTS)
    return vowels, consonants

if __name__ == "__main__":
    a = input("Enter a string: ")
    vowels, consonants = count_vowels_consonants(a)
    other = len(a.casefold()) - vowels - consonants
    print(f"{vowels} vowels, {consonants} consonants and {other} other characters")

Some other concepts which are useful and contained in this short function:

  • in is \$\mathcal{O}(n)\$ for list, tuple, str and \$\mathcal{O}(1)\$ for set and dict. Here it does not matter too much, because len(CONSONANTS) == 21.
  • Generator expressions are as easy to use as list comprehensions, but avoid creating a temporary list in memory. Useful if you do something with the items right away (like summing them up, as I am doing here).
  • f-strings, or format strings, are a nice way to easily format output.
  • Putting the calling code with the user in-/output behind a if __name__ == "__main__": guard allows you to import from this script without running the code.
  • Tuple unpacking let's you easily return multiple return values from a function and assign them to separate variables. This can get very fancy with advanced tuple unpacking.
  • You should always think about edge cases. Here there are several. Some characters convert to multiple characters when being case-folded (e.g. ß), for some characters it might depend on the locale if they are vowels, like äâàãá and some characters are definitely neither vowels, nor consonants, at least all of \t\n\x0b\x0c\r !"#$%&\'()*+,-./0123456789:;<=>?@[\\]^_{|}~. The last category is even larger than the standard alphabet. It contains whitespace, punctuation, digits and other miscellaneous symbols. You can get it with "".join(sorted(set(string.printable) - set(string.ascii_letters))). And this is just ASCII. Since Python strings are unicode strings, this list is actually quite a lot bigger.

    Your code currently treats all non-vowels as consonants, which is probably not correct, unless you guarantee that the user-entered string contains only ASCII letters. My code specifically counts the number of consonants via a whitelist of consonants. This way you can at least add a third category other.

\$\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.