Vowel/consonant counter in Golang

Question

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

Answer 1

• 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 =)

Answer 2

You should use a simple switch statement, like this:

func isVowel(r rune) bool {
    switch r {
    case 'a', 'e', 'i', 'o', 'u':
        return true
    }
    return false
}

This is much more efficient than creating a map each time.

Answer 3

Since the intended solution obviously has to process user input, I focused on the runes that are neither vowels nor consonants. I also found it confusing to iterate more than once, and I came to this solution. I'm an absolute Go beginner, so I'm currently not able to separate map building and analysis. But I'm thankful to this interesting experience practising Go's maps, loops, error handling strategy (double assign), and formatting capabilities:

package main

import "fmt"

type RuneTypeCounts struct {
  vowels, consonants, other int
}

func analyseText(text string) RuneTypeCounts {
  var result RuneTypeCounts
  m := make(map[rune]*int)
  for _, r := range "aeiou" {
    m[r] = &result.vowels
  }
  for _, r := range "bcdfghjklmnpqrstvwxyz" {
    m[r] = &result.consonants
  }
  for _, r := range text {
    ref, inMap := m[r]
    if inMap {
        *ref ++
    } else {
        result.other ++
    }
  }
  return result;
}

func main() {
  rt := analyseText("Hello, world!")
  fmt.Printf("%+v",rt)
}

---

edit: I extracted the analysing function, but the map rebuild per call makes multiple calls to the function inefficient. Some rewording (and typo fixing).