HiveBrain v1.2.0
Get Started
← Back to all entries
patterngoMinor

Vowel/consonant counter in Golang

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
vowelgolangcounterconsonant

Problem

To learn Go I decided to write a vowel/consonant counter. I really need tips/ideas on how I can improve my code.

package main

import (
    "os"
    "bufio"
    "fmt"
)

func isVowel(x string) bool {
    vowels := [5]string{"a", "e", "i", "o", "u",}
    vowelLookupTable := make(map[string]bool)
    for _, v := range vowels {
        vowelLookupTable[v] = true
    }

    return vowelLookupTable[x]
}

func isConsonant(x string) bool {
    consonants := [21]string{"b", "c", "d", "f", "g", "h", "j", "k", "l", "m", "n", "p", "q", "r", "s", "t", "v", "w", "x", "y", "z"}
    consonantLookupTable := make(map[string]bool)
    for _, v := range consonants {
        consonantLookupTable[v] = true
    }

    return consonantLookupTable[x]

}

func formatNumbers(vowelCount int, consonantCount int) {
    fmt.Print("Consonants: ")

    for i := 0; i < consonantCount; i++ {
        fmt.Print("=")
    }

    fmt.Printf(" %d\n", consonantCount)

    fmt.Print("Vowels:     ")

    for i := 0; i < vowelCount; i++ {
        fmt.Print("=")
    }

    fmt.Printf(" %d\n", vowelCount)
}

func countVowels(input string) int {
    vowelCount := 0

    for _, x:= range input {
        var s string
        s = fmt.Sprintf("%c", x)
        if isVowel(s) {
            vowelCount += 1
        }
    }

    return vowelCount
}

func countConsonants(input string) int {
    consonantCount := 0

    for _, x:= range input {
        var s string
        s = fmt.Sprintf("%c", x)
        if isConsonant(s) {
            consonantCount += 1
        }
    }

    return consonantCount
}

func takeInput() string {
    reader := bufio.NewReader(os.Stdin)
    fmt.Print("Enter text: ")
    text, _ := reader.ReadString('\n')

    return text
}

func main() {
    input := takeInput()

    vowelCount := countVowels(input)
    consonantCount := countConsonants(input)

    formatNumbers(vowelCount, consonantCount)
}

Solution


  • Handle errors. In Go, foo, _ := something() is a big code smell.



  • Iterating on a string gives you a rune; don't convert it to a one-char string just to compare it to another one-char string. Compare it to another rune instead ('a' instead of "a").



-
Your functions isVowel and isConsonant are re-building the vowel/consonant map from scratch every time they're called! This is very inefficient: initialize both maps as package variables, and inline these two functions completely. You have two options to do so, I like the second one better because it's slightly more readable.

Option 1:

var (
    vowels = map[rune]bool{
        'a': true,
        'e': true,
        'i': true,
        'o': true,
        'u': true,
    }
    consonants = map[rune]bool{
        // …
    }
)


Option 2:

var (
    vowels = toSet('a', 'e', 'i', 'o', 'u')
    consonants = toSet(/* … */)
)
func toSet(chars ...rune) map[rune]bool {
    m := make(map[rune]bool, len(chars))
    for _, c := range chars {
        m[c] = true
    }
    return m
}


You then check if a rune r is a vowel by just calling vowels[r].

-
Your s = fmt.Sprintf("%c", x) statement looks a lot like printf debugging. Were you intending to keep it in your code?

  • countVowels and countConsonants can be inlined as well, and called on each char in the same loop like in Curtis' answer.



  • Replace foo += 1 by foo++.



  • I'd say you have needless blank lines in your code (I think each of your functions is simple enough not to have any), but that's a minor style thing.



  • Your code doesn't support non-ASCII encodings (and it probably should, UTF-8 is everywhere nowadays). It doesn't support uppercase letters, either =)

Code Snippets

var (
    vowels = map[rune]bool{
        'a': true,
        'e': true,
        'i': true,
        'o': true,
        'u': true,
    }
    consonants = map[rune]bool{
        // …
    }
)
var (
    vowels = toSet('a', 'e', 'i', 'o', 'u')
    consonants = toSet(/* … */)
)
func toSet(chars ...rune) map[rune]bool {
    m := make(map[rune]bool, len(chars))
    for _, c := range chars {
        m[c] = true
    }
    return m
}

Context

StackExchange Code Review Q#149510, answer score: 5

Revisions (0)

No revisions yet.