1
\$\begingroup\$

The task is for training go-lang. The idea is to extract unique words sorted and grouped by length. Might be useful in learning new words. The program uses command line argument assuming it's a file to read; reads from stdin if no arguments were given.

package main

import (
    "bufio"
    "fmt"
    "os"
    "regexp"
    "sort"
    "strings"
)

type words = map[string]struct{}
type groupedWords = map[int]words

// Checks for fatal errors.
// If non nil error passed program will be terminated
func check(e error) {
    if e != nil {
        panic(e)
    }
}

// Reads data from bufio.Reader and returns
// a map of grouped unique words: key is letters count, value is map of words
func getGroupedWords(reader *bufio.Reader) groupedWords {

    groupedWords := make(groupedWords)

    re, err := regexp.Compile("[^a-zA-Z]")
    check(err)
    for {
        str, err := reader.ReadString(' ')
        str = strings.ToLower(strings.TrimSpace(str))
        if !re.MatchString(str) {
            val, exists := groupedWords[len(str)]
            if !exists {
                groupedWords[len(str)] = words{str: struct{}{}}
            } else {
                val[str] = struct{}{}
            }

        }

        if err != nil {
            break
        }
    }

    delete(groupedWords, 0)
    return groupedWords

}

// Extracts sorted slice of keys from words
func getSortedKeysWord(w words) []string {
    list := make([]string, len(w))

    i := 0
    for word := range w {
        list[i] = word
        i++
    }

    sort.Strings(list)
    return list
}

// Extracts sorted slice of letters count from groupedWords
func getSortedKeysGroupedWord(gw groupedWords) []int {
    list := make([]int, len(gw))

    i := 0
    for lettersCnt := range gw {
        list[i] = lettersCnt
        i++
    }

    sort.Ints(list)
    return list
}

func main() {
    var reader *bufio.Reader
    args := os.Args
    if len(args) == 1 {
        reader = bufio.NewReader(os.Stdin)
    } else {
        file, err := os.Open(args[1])
        check(err)
        reader = bufio.NewReader(file)
    }

    groupedWords := getGroupedWords(reader)

    lettersCntList := getSortedKeysGroupedWord(groupedWords)
    for _, lettersCnt := range lettersCntList {
        list := getSortedKeysWord(groupedWords[lettersCnt])
        fmt.Print(lettersCnt, "(", len(list), "):\t")
        for _, word := range list {
            fmt.Print(word, " ")
        }

        fmt.Println()
    }

}

usage

echo "The unary notation can be abbreviated by introducing different symbols for certain new values. " | go run extract_words.go

should give

2(2):   be by
3(3):   can for new
5(1):   unary
7(2):   certain symbols
8(1):   notation
9(1):   different
11(2):  abbreviated introducing
\$\endgroup\$

2 Answers 2

2
\$\begingroup\$

If code is not correct it is not useful and it may be harmful.


I reviewed the specification for your program,

The idea is to extract unique words sorted and grouped by length.

the sample input,

echo "The unary notation can be abbreviated by introducing different symbols for certain new values. " | go run extract_words.go

and the sample output.

2(2):   be by
3(3):   can for new
5(1):   unary
7(2):   certain symbols
8(1):   notation
9(1):   different
11(2):  abbreviated introducing

I then ran your program

2(2):   be by 
3(4):   can for new the 
5(1):   unary 
7(2):   certain symbols 
8(1):   notation 
9(1):   different 
11(2):  abbreviated introducing 

felix posted an answer, so I ran his program

2(2):   be by
3(4):   can for new the
5(1):   unary

Three different outputs! All wrong!


None of the earlier outputs is correct. I wrote a program to find the correct output.

Output:

2(2):   be by
3(4):   can for new the
5(1):   unary
6(1):   values
7(2):   certain symbols
8(1):   notation
9(1):   different
11(2):  abbreviated introducing

wordsbylen.go:

package main

import (
    "bufio"
    "bytes"
    "fmt"
    "io"
    "os"
    "sort"
    "strings"
    "unicode"
)

func wordsByLen(r io.Reader) ([][]string, error) {
    unique := make(map[string]bool)
    scan := bufio.NewScanner(r)
    for scan.Scan() {
        fields := bytes.FieldsFunc(
            scan.Bytes(),
            func(r rune) bool {
                return !unicode.IsLetter(r)
            },
        )
        for _, field := range fields {
            unique[string(bytes.ToLower(field))] = true
        }
    }
    if err := scan.Err(); err != nil {
        return nil, err
    }

    words := make([]string, 0, len(unique))
    for word := range unique {
        words = append(words, word)
    }

    sort.Slice(words,
        func(i, j int) bool {
            if len(words[i]) < len(words[j]) {
                return true
            }
            if len(words[i]) == len(words[j]) {
                return words[i] < words[j]
            }
            return false
        },
    )

    var byLen [][]string
    for i, j := 0, 1; j <= len(words); j++ {
        if j == len(words) || len(words[j]) != len(words[i]) {
            byLen = append(byLen, words[i:j])
            i = j
        }
    }
    return byLen, nil
}

func main() {
    f := os.Stdin
    if len(os.Args) == 2 {
        var err error
        f, err = os.Open(os.Args[1])
        if err != nil {
            fmt.Fprintln(os.Stderr, err)
            return
        }
        defer f.Close()
    }

    byLen, err := wordsByLen(f)
    if err != nil {
        fmt.Fprintln(os.Stderr, err)
        return
    }
    for _, words := range byLen {
        if len(words) > 0 {
            list := strings.Join(words, " ")
            fmt.Printf("%d(%d): \t%s\n", len(words[0]), len(words), list)
        }
    }
}

Go programmers usually like to write efficient code. I ran the code on Shakespeare:

The Complete Works of William Shakespeare by William Shakespeare
http://www.gutenberg.org/files/100/100-0.txt

For my program (correct results)

real    0m0.369s
user    0m0.356s
sys     0m0.012s

For your program (incorrect results)

real    0m0.675s
user    0m0.630s
sys     0m0.040s

For felix's program (incorrect results)

real    1m58.704s
user    1m58.600s
sys     0m0.140s

Since felix's program takes hundreds of times longer than other programs, it is unlikely to be useful.


You and felix have a narrow view of words, limiting them to ASCII letters

regexp.Compile("[^a-zA-Z]")

There is a larger world out there: The Unicode Consortium. For example, unicode.IsLetter()


The Go garbage collector manages memory, it does not manage operating system resources like files. To avoid crashing with too many open files, get into the habit of releasing OS file resources by closing files when no longer needed. See my use of defer f.Close().


Two key goals of the Go programming language are simplicity and readability.

Your text algorithms favor regular expressions, which can be complicated and error prone. See Mastering Regular Expressions, 3rd Edition, Jeffrey Friedl, ISBN-13: 978-0596528126.

Your data structures are complicated, favoring maps (dictionaries). For example, map[int]map[string]struct{}. I used simple structures: map[string]bool, []string, and, at the end, [][]string.

Your control structures look complicated. I used a simple, sequential control structure: unique words -> word list -> sort word list -> group word list -> print grouped word list.

\$\endgroup\$
1
  • \$\begingroup\$ thanks for pointing out the mistake in my answer ! I definitely wasn't paying enough attention. And you're totally right about the unicode.IsLetter() method, it's way better than the regex. \$\endgroup\$
    – felix
    Sep 9, 2018 at 2:00
1
\$\begingroup\$

This program could be improved by using more specific tools from the standard Library

Use a Scanner

The best way to extract words from a text is to use a bufio.Scanner with the split function bufio.ScanWords ( see this example in godoc)

The for loop in getGroupedWords could be rewritten like this :

scanner := bufio.NewScanner(file)
scanner.Split(bufio.ScanWords)
for scanner.Scan() {
    str := strings.ToLower(scanner.Text())
    ... 
}

Use a sort.StringSlice

Instead of having two map words and groupedWords, we could have just a single map of type map[int]sort.StringSlice. sort.StringSlice provides a Sort() method and a Search() method to check if the slice already contains a specific string.

The content of the previous for loop could be this:

str := strings.ToLower(scanner.Text())
if !re.MatchString(str) {
    wordList := words[len(str)]
    wordList.Sort()
    i := wordList.Search(str)
    if i < len(wordList) && str == wordList[i] {
        // the word is already present in the slice
    } else {
        words[len(str)] = append(wordList, str)
    }
}

Use fmt.Printf instead of fmt.Print

The method fmt.Printf is intended to print strings in a specific format. This help improving readability:

in the main() function, the part

lettersCntList := getSortedKeysGroupedWord(groupedWords)
for _, lettersCnt := range lettersCntList {
    list := getSortedKeysWord(groupedWords[lettersCnt])
    fmt.Print(lettersCnt, "(", len(list), "):\t")
    for _, word := range list {
        fmt.Print(word, " ")
    }
    fmt.Println()
}

could be rewritten like this :

for i := 0; i < len(groupedWords); i++ {
    if wordList, ok := groupedWords[i]; ok {
        wordList.Sort()
        fmt.Printf("%d(%d):\t%s\n", i, len(wordList), strings.Join(wordList, " "))
    }
}

Details

  • A regex can be defined as global variable with regex.MustCompile, so we don't need to check for error. If the regex is incorrect, the program won't build
  • avoid method like check(err error) and handle error locally instead

The final program :

package main

import (
    "bufio"
    "fmt"
    "os"
    "regexp"
    "sort"
    "strings"
)

var re = regexp.MustCompile("[^a-zA-Z]")

func groupedWords(file *os.File) (map[int]sort.StringSlice, int) {

    var words = map[int]sort.StringSlice{}
    maxSize := 0

    scanner := bufio.NewScanner(file)
    scanner.Split(bufio.ScanWords)
    for scanner.Scan() {
        str := strings.ToLower(scanner.Text())

        if len(str) > maxSize {
            maxSize = len(str)
        }

        if !re.MatchString(str) {
            wordList := words[len(str)]
            wordList.Sort()
            i := wordList.Search(str)
            if i < len(wordList) && str == wordList[i] {
                // the word is already present in the slice
            } else {
                words[len(str)] = append(wordList, str)
            }
        }
    }
    return words, maxSize
}

func main() {

    var input = os.Stdin

    args := os.Args
    if len(args) > 1 {
        f, err := os.Open(args[1])
        if err != nil {
            panic(err)
        }
        input = f
    }

    groupedWords, maxSize := groupedWords(input)

    for i := 0; i <= maxSize; i++ {
        if wordList, ok := groupedWords[i]; ok {
            wordList.Sort()
            fmt.Printf("%d(%d):\t%s\n", i, len(wordList), strings.Join(wordList, " "))
        }
    }
}
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.