DEV Community

loading...

Discussion on: Attempting to Learn Go - Building Dev Log Part 03

Collapse
ladydascalie profile image
Benjamin Cable • Edited

About this snippet:

func getContents(f *string) ([]byte, error) {
  b, err := ioutil.ReadFile(*f)
  if err != nil {
    return nil, err
  }
  return b, nil
}

You're taking in *string when string is better for what you want to do.

But more importantly, you are not checking your pointer:

if f == nil {
    return nil, errors.New("contextual error about f being nil")
}

I'll make the assumption that you are doing it this way because you are already checking before the function, but I would say that this would be better done inside so you never have to think about it again.

On another note: I don't think the function expresses what it's doing correctly. Consider the following alternative signatures:

  • func getFileContents(filename string) ([]byte, error) {}
  • func contentsOf(f *os.File) ([]byte, error) {}
  • func contentsOf(r io.Reader) ([]byte, error) { return ioutil.ReadAll(r) }

And in that last case, if you're operating on readers, why not just... not use a function at all!

As an *os.File is a Reader, you would eliminate the boilerplate code, and be able to leverage one of the most powerful interfaces in the standard library.

Collapse
shindakun profile image
Steve Layton Author

Thanks for the comments! I really should be passing in just the string as you've pointed out, there is no reason to use the pointer. My thinking in making this a function on its own was to be able to write tests for it later, which I suppose means I had better be checking for that pointer. I'm not sure just wrapping reading a file into a function is worth it just to be able to write a test down the road though so it might be worth doing it away with it entirely.

Definitely, something for me to consider. Thanks again!

Collapse
ladydascalie profile image
Benjamin Cable

Worth considering as well that using a Reader rather than a concrete file here will make testing this easier later anyway, since you'll be able to use any Reader. not just a file, you can mock your input using buffers or any old strings.NewReader("thing")!

Thanks for taking the time to respond.

Thread Thread
shindakun profile image
Steve Layton Author

That is a fantastic point! I think using a Reader is going to be the way I go.