0
\$\begingroup\$

I've just started learning Golang and was solving some problems which required me to read lines of text from a file. I decided to abstract away the reading part so that I can use Go's range keyword to iterate over lines of any "file".

Here is my attempt:

import (
    "bufio"
    "fmt"
    "io"
    "io/fs"
    "os"
    "reflect"
)

func ReadLines[F string | *fs.File](fileOrName F) chan string {
    ch := make(chan string, 10)
    var file io.Reader

    switch fon := reflect.ValueOf(fileOrName); fon.Kind() {
    case reflect.String:
        if _file, err := os.Open(fon.String()); err != nil {
            panic(fmt.Sprintf("Could not open file: '%s'. Error: %v", fileOrName, err))
        } else {
            file = _file
            //goland:noinspection GoUnhandledErrorResult
            defer _file.Close()
        }
    case reflect.Pointer:
        file = *(*fs.File)(fon.UnsafePointer())
    }

    fileScanner := bufio.NewScanner(file)
    go func() {
        defer close(ch)
        for fileScanner.Scan() {
            ch <- fileScanner.Text()
        }
    }()
    return ch
}

In particular, I would like some feedback on the following:

  • Pointers: My knowledge on how pointers work comes from C++ and C, so I would like to know if my code has an accent that may still be influenced by my C background, or if I've done it right.
  • Generics and reflect package: I was hoping Golang supported multiple dispatch, but from my reading, Go is a structurally typed language, so multiple dispatch doesn't work. This is why I used generics to emulate the behavior. I also use reflection to determine the type of the parameter passed in.

Any other feedback is appreciated.

Golang version: 1.19.4

\$\endgroup\$

1 Answer 1

4
\$\begingroup\$

A few problems stand out right away in your solution. One is that you can't write a union of types with | in the type list of a generic function. Another is that fs.File is an interface type, and pointers to interfaces are seldom needed or used in Go. Another is that defer _file.Close() will fire as soon as ReadLines returns, while the goroutine is still trying to read from the file. (You want to close the file only after you're done reading from it, which is when the goroutine finishes.)

It would be simpler, more general, and more idiomatic to write your function in terms of io.Reader, requiring:

  • callers with filenames to do their own os.Open (and their own Close);
  • callers reading from strings to wrap them with strings.NewReader(s) before calling your function;
  • callers reading from []byte slices to wrap them with bytes.NewReader(s);
  • callers with fs.FS objects to call Open on them;
  • etc.

Note that fs.File already is an io.Reader.

// ReadLines produces a channel of the lines scanned from r.
// It also produces an error-returning function.
// The caller may call that after consuming the channel
// in order to discover any error
// that may have been encountered during the scan.
func ReadLines(ctx context.Context, r io.Reader) (<-chan string, func() error) {
  var (
    ch  = make(chan string)
    sc  = bufio.NewScanner(r)
    err error
  )
  go func() {
    defer close(ch)
    for sc.Scan() {
      line := sc.Text()
      select {
      case <-ctx.Done():
        err = ctx.Err()
        return
      case ch <- line:
      }
    }
    err = sc.Err()
  }()
  return ch, func() error { return err }
}

Things to note in this code:

  • The function takes a context.Context, which is a good practice when you're spawning a long-lived goroutine. It gives a clean and idiomatic way to clean up resources when the caller needs to impose a timeout or otherwise cancel.
  • The function returns a <-chan, not a chan, because the caller should be allowed only to read from it, not write to it.
  • The function also returns a func() error that produces any error that may have been encountered in the goroutine.
  • The channel is unbuffered. A rule of thumb in Go is that channels should be unbuffered or have a buffer size of 1. Any other number is usually either a premature optimization or simply unnecessary. In this case, writing to the unbuffered channel will cause this goroutine to block until some other goroutine is ready to read from the channel, which will wake this goroutine back up, and that's just fine.
  • A select statement chooses the first channel operation that can proceed: either detecting the context object's cancellation, or sending the next line on the output.

I'll point out that all of this is exactly how iter.Lines behaves (in my "go-generics" module), except that the output channel and the deferred error-reading function are encapsulated in the iter.Of[string] interface. The same caveat mentioned there applies here: bufio.Scanner imposes a maximum line length that can cause panics when exceeded. You can work around this by using your own buffer, see Scanner.Buffer.

If you're dead set on doing type dispatch, you don't need generics or unsafe pointers. This should work:

func ReadLines(ctx context.Context, arg any) (<-chan string, func() error) {
  var (
    r io.Reader
    c io.Closer
  )
  switch arg := arg.(type) {
  case io.Reader:
    r = arg
  case string: // presumably a filename
    f, err := os.Open(arg)
    if err != nil {
      // Note, callers must now be prepared to get back a nil channel.
      return nil, func() error { return err }
    }
    r, c = f, f
  default:
    return nil, func() error { fmt.Errorf("unsupported arg type %T", arg) }
  }

  // The rest is mostly like my first version above,
  // except that there is now also an io.Closer
  // to make sure we close anything we opened,
  // but not until we're done reading from it.

  var (
    ch  = make(chan string)
    sc  = bufio.NewScanner(r)
    err error
  )
  go func() {
    defer close(ch)
    if c != nil {
      defer c.Close()
    }
    for sc.Scan() {
      line := sc.Text()
      select {
      case <-ctx.Done():
        err = ctx.Err()
        return
      case ch <- line:
      }
    }
    err = sc.Err()
  }()
  return ch, func() error { return err }
}
```
\$\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.