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:
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.
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!
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 anyReader. not just a file, you can mock your input using buffers or any old strings.NewReader("thing")!
About this snippet:
You're taking in
*stringwhenstringis better for what you want to do.But more importantly, you are not checking your pointer:
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.Fileis aReader, you would eliminate the boilerplate code, and be able to leverage one of the most powerful interfaces in the standard library.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!
Worth considering as well that using a
Readerrather than a concrete file here will make testing this easier later anyway, since you'll be able to use anyReader. not just a file, you can mock your input using buffers or any oldstrings.NewReader("thing")!Thanks for taking the time to respond.
That is a fantastic point! I think using a
Readeris going to be the way I go.