Update - How do you refactor this piece of code?

Hello fellow gophers, I need some insights. I'm writing a command line client for larder.io bookmark in go. I have this function:

func checkConfigRoute(configDir string) (string, error) {
    usr, err := user.Current()
    if err != nil {
        return nil, err
    }
    route := join(usr.HomeDir, "/", configDir)
    // Check if the directory exist, if not, create it the route.
    if _, err := os.Stat(route); os.IsNotExist(err) {
        if err := os.MkdirAll(route, os.ModePerm); err != nil {
            return nil, err
        }
    }
    return route, nil
}

I need to check if a config file exist, but before that I want to check if the route directory exist, if not create it and return the route.

I don't know why but it feels off, not sure if I'm doing it wrong. What do you think? How do you refactor it? Should I pass the full route directly to the function instead of joining it inside? Help much appreciated!

Thanks!

UPDATE:

Okey, let's see. I've got rid of that function and instead reworked the whole config setup.

Here is the code:

// Config token
// and path
// yaml config file
type Config struct {
    FileName    string
    Path        string
    TokenString string `yaml:"token"`
}

// Token returns the configuration token string
func (c Config) Token(handle []byte) (string, error) {
    err := yaml.Unmarshal(handle, &c)
    if err != nil {
        log.Println("Error: ", err)
    }
    return c.TokenString, err
}

// SetPath defines the configuration directory path
func (c Config) SetPath(condigDir string) {
    c.Path = condigDir
}

func getUsrHomeDir() string {
    usr, err := user.Current()
    if err != nil {
        log.Println("User home error: ", err)
    }
    return join(usr.HomeDir, "/")
}

func firstTimeRun(path string) error {
    // Since we are going to store our api token,
    // for security reasons we don't want give read access
    // to other users in our system.
    if err := os.MkdirAll(path, 0700); err != nil {
        return err
    }

    return nil
}

func readConfig(route string) []byte {
    handle, err := ioutil.ReadFile(route)

    // Most common cases are:
    // Either the file doesn't exist, or
    // Exist but you don't have read permission

    if os.IsNotExist(err) {
        firstTimeRun(route)
    }

    if os.IsPermission(err) || err != nil {
        log.Println("Reading config file: ", err)
        os.Exit(1)
    }
    return handle
}

Because ioutil.ReadFile already return any error if there is a problem with the config file, I can directly avoid the whole checkConfigRoute, If there is an error I already know that there is something wrong. So I initalizing it from the readConfig function instead.

Thoughts?

Thanks again!

Comments

[Orlando Brown]

Not that experienced in Golang, but your heading in the right direction to split it out into separate functions. Always much easier reason with and test when each function does one thing.

[Florian Rand]

Thanks Orlando!