patterngoMinor
Vowel/consonant counter in Golang
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 anotherruneinstead ('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?countVowelsandcountConsonantscan be inlined as well, and called on each char in the same loop like in Curtis' answer.
- Replace
foo += 1byfoo++.
- 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.