HiveBrain v1.2.0
Get Started
← Back to all entries
snippetgoMinor

Create a new unique file

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
uniquefilenewcreate

Problem

For debugging purposes I am storing requests into files. This code is supposed to create a new file for each request.

Somehow it doesn't look very nice, can it be improved? Additionally it would be nice if it could be made atomic.

filenamePrefix := exeDir + "/debug/" + time.Now().Format("20060102150405")
var filename string

for i := 0 ; ; i++ {
    if (i > 100) {
        log.Print("Could not find free filename")
        return
    }

    filename = filenamePrefix + "-" + strconv.Itoa(i)
    _, err := os.Stat(filename);
        if os.IsNotExist(err) {
        break
    }
}

fd, err := os.Create(filename)
if err != nil {
    log.Print(err.Error())
    return
}
defer fd.Close()

io.Copy(fd, request.Body)

Solution

Your code has a few items in it which concern me in a few small ways, and, in general, while I think your code is OK functionally, it could be done better (I am biased, I have done similar things, and ended up doing it differently).

First up, my concerns...

Timestamp function

while it is a 1-liner, I would still have a method to encapsulate the time formatting:

func buildFileName() string {
    return time.Now().Format("20060102150405")
}


filepath

You should use path/filepath to manage your path names:

import (
  "path/filepath"
)

filenamePrefix := filepath.Join(exeDir, "debug", buildFileName())


Functions

You need more functions... still. I would create a function to control the file creation process (note the use of %d instead of string.Itoa...):

func pickFileName(prefix string) (string, error) {
    for i := 0; i < 100; i++ {
        fname := fmt.Sprintf("%s_%d", prefix, i)
        if _, err := os.Stat(fname); os.IsNotExist(err) {
            return fname, nil
        }
    }
    return "", fmt.Errorf("Unable to create a unique file with the prefix %v in 100 tries", prefix)
}


Result

OK, now your code looks like:

filenamePrefix := filepath.Join(exeDir, "debug", buildFileName())
filename, err := pickFileName(filenamePrefix)
if err != nil {
    log.Print(err)
    return
}

fd, err := os.Create(filename)
if err != nil {
    log.Print(err)
    return
}
defer fd.Close()

io.Copy(fd, request.Body)


Error handling

You have log.Print(err.Error()) but log.Print(err) is fine. Still, you should probably be returning an error, instead of just logging it....

Alternative

If it were me, I would do two things.....

First up, create a directory for the actual process you are running as.

myid := fmt.SPrintf("Process_%d", os.Getpid())


then, ensure you use that in your path:

filenamePrefix := filepath.Join(exeDir, "debug", myid, buildFileName())


Now, you have a folder dedicated to the current process (and collisions with other running instances of your process are impossible - at the same time - though there may be an old instance of that same folder for an older process - that's OK).

Then, create a counter (a "global") too:

var debugCounter uint64


And a matching function:

func nextDebugId() string {
    return fmt.Sprintf("%d", atomic.AddUint64(&debugCounter, 1))
}


Now, you can simplify the whole thing (by modifying buildFileName) to:

func buildFileName() string {
    return time.Now().Format("20060102150405") + "_" + nextDebugId()
}


and your whole code then becomes:

fullpath := filepath.Join(exeDir, "debug", myid, buildFileName())

fd, err := os.Create(fullpath)
.....

Code Snippets

func buildFileName() string {
    return time.Now().Format("20060102150405")
}
import (
  "path/filepath"
)

filenamePrefix := filepath.Join(exeDir, "debug", buildFileName())
func pickFileName(prefix string) (string, error) {
    for i := 0; i < 100; i++ {
        fname := fmt.Sprintf("%s_%d", prefix, i)
        if _, err := os.Stat(fname); os.IsNotExist(err) {
            return fname, nil
        }
    }
    return "", fmt.Errorf("Unable to create a unique file with the prefix %v in 100 tries", prefix)
}
filenamePrefix := filepath.Join(exeDir, "debug", buildFileName())
filename, err := pickFileName(filenamePrefix)
if err != nil {
    log.Print(err)
    return
}

fd, err := os.Create(filename)
if err != nil {
    log.Print(err)
    return
}
defer fd.Close()

io.Copy(fd, request.Body)
myid := fmt.SPrintf("Process_%d", os.Getpid())

Context

StackExchange Code Review Q#132025, answer score: 7

Revisions (0)

No revisions yet.