3
\$\begingroup\$

I'm beginning to learn Go. I've decided to write some file utilities to help me with some bigger projects I intend to write. Coming from mainly a Python background, I'm sure there are plenty of stylistic and efficiency improvements to be made. I'm looking for general feedback, as I want to kick bad habits to the curb before they set it. Feedback such as:

  • Error Reporting
  • Use of defer in appendContent function
  • Testing Functions: Are there any libraries to help accomplish this, like how Python has unittest?

I use more comments than I usually would when programming because I'm still learning the language. The Usage: comments help me remember why I'm importing these libraries. Besides the specifically requested feedback, any and all criticism is welcome and appreciated!

Utilities

package main

import (
    "fmt"       // Usage: Printing errors and content of the files
    "io/ioutil" // Usage: Reading and writing files
    "os"        // Usage: Opening files and appending to them
)

func main() {
    testFunctions()
}

func testFunctions() {
    /*
        This method is solely for the purpose of testing the below methods
    */
    writeContent("test.txt", "Test Content")
    fmt.Println(getContent("test.txt"))
    appendContent("test.txt", "Test Content 2")
    fmt.Println(getContent("test.txt"))
}

func getContent(file string) string {
    /*
        Gets the content from the passed `file`
    */
    content, err := ioutil.ReadFile(file)
    if err != nil {
        fmt.Println(err)
        return "[NOT CONTENT]: AN ERROR OCCURED"
    }
    return string(content)
}

func writeContent(file string, content string) {
    /*
        Writes the passed `content`, creating a new file at `file`
    */
    formattedContent := []byte(content)
    err := ioutil.WriteFile(file, formattedContent, 0777)
    if err != nil {
        fmt.Println(err)
    }
}

func appendContent(file string, content string) {
    /*
        Appends the passed `content` to the end of the passed `file`
    */
    fileToAppend, err := os.OpenFile(file, os.O_APPEND|os.O_WRONLY, 0600)
    if err != nil {
        panic(err)
    }
    defer fileToAppend.Close()
    _, err = fileToAppend.WriteString(content)
    if err != nil {
        panic(err)
    }
}
\$\endgroup\$
2
\$\begingroup\$

Regarding testing: Yes, it's in the standard library. There are of course more libraries to help out more, but that's where you should likely start your journey.


For error reporting, well, it's a bit inconsistent at the moment? There's both returning a value ([NOT CONTENT]...) as well as fmt.Println and panic too, all of which are not very appropriate for production usage.

The first of which can lead to serious debugging nightmares: How do you know the file didn't contain that exact string for one? This is always a problem if the valid values are mixed with error values (like how in most C APIs there are int return values with -1 for errors, while everything \$>= 0\$ are the "actual" return values in the non-error case)! It's much better to keep them separate, which in Go could be done with separate types, or separate return values (which is what's done for errors).

The second one is perhaps less bad, but again, the error situation is just hidden, not actually dealt with.

The third one can be done, however it's usually reserved for programming errors, since not catching a panic call will simply abort the program (which is rarely what you want).

If in doubt, return an error in addition to any values: func getContent(string) (string, error) and writeContent/appendContent(string, string) error would be fine. Maybe also see this blog post and this newer one for 1.13.


The defer to close files is pretty much required in my opinion. Unless you can prove that nothing else between the opening of the file and the Close call can raise a panic, you pretty much have to use defer ...Close(), otherwise you risk leaking resources (open file handles).

For the comments, please have a look here: Instead of comments simply add proper docstrings to the functions, that is good practice anyway and would certainly be appreciated by colleagues on bigger projects.

Maybe as a last note: The permissions 0777 and 0600 are okay if those are the ones you want. However, that might not be true in general. Exactly how ioutil.WriteFile lets you specify them would be likely what you want in general. If not, consider creating constants so that you don't have magic numbers inline with the code (e.g. const defaultFilePermissions = 0777 or something similar).

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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