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!

Blog

Update - How do you refactor this piece of code?

Florian Rand

UPDATE:

Join Our Newsletter. No Spam, Only the good stuff.

Related