Idiomatic Go
Joey Geralnik
I'm working on a university assignment. Every person got a different (random) programming language to do a series of problems in, and I got Go. I finished and handed in the first problem, but I suspect my program is overly ugly and not at all idiomatic. Since the teacher doesn't know Go, and so won't be grading based on style, I though I would post my work here and ask for corrections as to the proper way to solve this problem in Go.
In particular, I felt that I needed a ridiculous amount of imports to accomplish basic things.
Here is the problem:
1) Input directory from the user.
2) Iterate over files in the directory. For the file hello.in, copy its contents line by line to the file hello.out. Print out the lines which include the word "you". For the rest of the files, insert a number at the beginning of each file which increases after each file.
My solution also has a problem that if a line in hello.in contains the word you and is longer than the size of the buffer, I have no way to print the whole line since I need to reuse the line variable for the next segment of a line. If there is a better way to do this, please let me know
In short, attached is my code. Any advice on how to make it simpler or better would be appreciated.
--Joey
Joey Geralnik
In short, attached is my code. Any advice on how to make it simpler or better would be appreciated.
--Joey
package main
import (
"bufio"
"fmt"
"io"
"io/ioutil"
"os"
"path/filepath"
"strconv"
"strings"
)
func walk(root string) filepath.WalkFunc {
count := 0
return func(path string, info os.FileInfo, err error) error {
if info.IsDir() {
if path == root {
return nil
}
return filepath.SkipDir
}
if path == root+"/hello.in" {
file, err := os.Open(path)
if err != nil {
return err
}
reader := bufio.NewReader(file)
output, err := os.Create(root + "/hello.out")
if err != nil {
return err
}
var (
isPrefix bool
line []byte
)
for err == nil {
line, isPrefix, err = reader.ReadLine()
output.Write(line)
if strings.Contains(string(line), "you") {
fmt.Println(string(line))
}
if !isPrefix {
output.WriteString("\n")
}
}
if err != io.EOF {
return err
}
file.Close()
output.Close()
return nil
}
result, err := ioutil.ReadFile(path)
if err != nil {
return err
}
file, err := os.OpenFile(path, os.O_WRONLY, 0666)
if err != nil {
return err
}
file.WriteString(strconv.Itoa(count))
file.Write(result)
file.Close()
count++
return nil
}
}
func main() {
var root string
//fmt.Scanln(&root)
root = "/home/joey/goProject/inputs"
err := filepath.Walk(root, walk(root))
if err != nil {
fmt.Println(err)
}
}
Andrew Gerrand
>
> On Tue, Mar 13, 2012 at 12:46 AM, Joey Geralnik <jger...@gmail.com> wrote:
>>
>> In short, attached is my code. Any advice on how to make it simpler or
>> better would be appreciated.
>> --Joey
>
>
> Sorry, here is the code inline for those who prefer it that way:
>
> package main
>
> import (
> "bufio"
> "fmt"
> "io"
> "io/ioutil"
> "os"
> "path/filepath"
> "strconv"
> "strings"
> )
Why do you feel this is a "ridiculous amount of imports"? Seems
reasonable to me.
You could definitely split parts of this function up into separate
functions. The part that handles hello.in is a good candidate.
> func walk(root string) filepath.WalkFunc {
> count := 0
> return func(path string, info os.FileInfo, err error) error {
> if info.IsDir() {
> if path == root {
> return nil
> }
> return filepath.SkipDir
> }
>
> if path == root+"/hello.in" {
> file, err := os.Open(path)
> if err != nil {
> return err
> }
>
> reader := bufio.NewReader(file)
>
> output, err := os.Create(root + "/hello.out")
> if err != nil {
> return err
> }
>
> var (
> isPrefix bool
> line []byte
> )
> for err == nil {
> line, isPrefix, err = reader.ReadLine()
You should check this error here, line may be something arbitrary if err != nil.
David Symonds
> file, err := os.Open(path)
> if err != nil {
> return err
> }
It is more common to follow this kind of code with an immediate "defer
file.Close()", rather than doing it after everything. In fact, your
code is omitting to close the file in any of the error cases, which a
defer would solve.
> if strings.Contains(string(line), "you") {
> fmt.Println(string(line))
> }
I'd use bytes.Contains instead of doing string conversions.
Other than those two things, it looks reasonable to me.
Dave.
Joey Geralnik
> if strings.Contains(string(line), "you") {I'd use bytes.Contains instead of doing string conversions.
> fmt.Println(string(line))
> }
fmt.Println(string(line))
}
The print still needs line to be converted to a string so that I don't get an array of ascii codes, right?
--Joey
David Symonds
> if bytes.Contains(line, []byte("you")) {
> fmt.Println(string(line))
> }
>
> The print still needs line to be converted to a string so that I don't get
> an array of ascii codes, right?
Yeah, or you could write the bytes directly.
os.Stdout.Write(line)
os.Stdout.Write([]byte{'\n'})
but converting there is fine.
Dave.
DisposaBoy
David Symonds
On Mar 13, 2012 6:58 PM, "DisposaBoy" <dispo...@dby.me> wrote:
> no, fmt.Printf("%s\n", line) should do it
Yeah, that works too. I don't think it's quite as efficient, though.
Dave.