6
\$\begingroup\$

To try and track down a troublesome AS400 bug, my company wants me to keep track of the order in which files are put into a folder. I spent an hour whipping something up in Go, but I'm still pretty new to Go so I'd like an opinion on a couple points:

  • Idiomatic Go: how do I make it more "Go"-like?
  • My method of freezing os.FileInfo objects

 

package main

import (
    "flag"
    "fmt"
    "io/ioutil"
    "log"
    "os"
    "time"
)

var POLLTIME = 10 * time.Second

type watchinfo struct {
    head     string      // path/to/the/
    fileinfo os.FileInfo // file
    seen     time.Time
}

// freeze the os.FileInfo to be stored in a map[miniFileInfo]bool
type miniFileInfo struct {
    name string
    size int64
}

func watchFolder(dirname string, ch chan<- watchinfo) {
    set := make(map[miniFileInfo]bool)
    for {
        fis, err := ioutil.ReadDir(dirname)
        if err != nil {
            panic("I failed to read " + dirname)
        }
        for _, fi := range fis {
            minifi := miniFileInfo{name: fi.Name(), size: fi.Size()}
            if _, ok := set[minifi]; !ok {
                // fi doesn't exist in the set, add it
                set[minifi] = true
                now := time.Now()
                // and send its watchinfo to the channel
                ch <- watchinfo{head: dirname, fileinfo: fi, seen: now}
            }
        }
        time.Sleep(POLLTIME) // let's not spam the server
    }
}

// WatchFolders creates a goroutine for each folder to watch
func WatchFolders(dirnames []string, outch chan<- string) {
    inch := make(chan watchinfo)
    for _, dir := range dirnames {
        go watchFolder(dir, inch)
    }
    for watch := range inch {
        outch <- fmt.Sprintf("%s\\%s with modified time %s", watch.head, watch.fileinfo.Name(), watch.fileinfo.ModTime().Format("Jan 2 15:04:05"))
    }
}

func main() {
    flag.Parse()
    ch := make(chan string)
    go WatchFolders(flag.Args(), ch)
    for seenat := range ch {
        log.Println(seenat)
    }
}

My original problem seemed to stem from using os.FileInfo objects directly. Every polling period would show all the files as "new." I'm not sure how Go handles identity in maps (in Python I could implement __hash__ to tell it how to behave) but as long as the file has the same name and the same size in bytes, I don't mind assuming it's the same file.

I overengineered a bit to allow for future use, allowing for multiple directories (I only need one) and keeping the first seen time in the struct (I can just use the log time for now).

\$\endgroup\$
1
\$\begingroup\$

Your code looks ok to me, but here are a couple of points to improve:

You used the flag package to obtain the arguments to your program. The original intention of the flag package is to process optional, named arguments to your program and store their parsed values into variables (designated by pointers). Using it just to obtain a list of arguments is somewhat "overkill", better would be just to use a simple variable in the os package: os.Args. os.Args holds the command-line arguments, starting with the program name. Since you don't need the program name, skip that by re-slicing this slice. Your new main() function:

func main() {
    ch := make(chan string)
    go WatchFolders(os.Args[1:], ch)
    for seenat := range ch {
        log.Println(seenat)
    }
}

By doing this we also got rid of the flag package dependency (so it won't have to be added to your compiled executable; and os was already used, so that's not a plus).

Next on to your map utilized as a Set. This is how you check if something is in your map:

if _, ok := set[minifi]; !ok {
    // fi doesn't exist in the set, add it
}

This works, but can be written simpler. Note that if a key is not in the map, indexing a map yields the zero value of the value type of the map (Spec: Index expressions). Zero value for the bool type is false. So the expression set[minifi] will be true if minifi is in the map (because when you put it into the map, you stored true as the value for this key), and it will be false if minifi is not in the map (the zero value of bool). So this can be rewritten as follows:

if !set[minifi] {
    // fi doesn't exist in the set, add it
}

Now an explanation why your original os.FileInfo didn't work as the key. When you want to find an element in a map by key, Go uses the == comparison operator to test for equality of keys (hash code is generated and used under the hood to speed up the lookups, but that is irrelevant now).

This is how the spec defines the terms and the result of the comparision. Quoting the relevant part from Spec: Comparision operators:

  • Struct values are comparable if all their fields are comparable. Two struct values are equal if their corresponding non-blank fields are equal.
  • Interface values are comparable. Two interface values are equal if they have identical dynamic types and equal dynamic values or if both have value nil.
  • Pointer values are comparable. Two pointer values are equal if they point to the same variable or if both have value nil. Pointers to distinct zero-size variables may or may not be equal.

Your miniFileInfo type is a struct, and 2 values of the same struct type are equal if all their fields are comparable and are equal. So you will find a value in the map if they match by name and size.

os.FileInfo is an interface, so a different comparison rule applies here. At runtime interface values can hold values of different dynamic type. Both the dynamic type and the value must match in order to claim 2 interface values equal. (You can read more about the representation of interface values in blog post The Laws of Reflection: The representation of an interface.)

The dynamic type of the values stored os.FileInfo interface is a pointer, a pointer to the unexported type os.fileStat. So now we need to apply the comparison rule for Pointers. 2 pointers are equal if they point to the same variable.

So 2 values of os.FileInfo will match only if the pointers stored in them match. You used ioutil.ReadDir() to obtain the slice of FileInfos, and whenever you call it, it creates new os.fileStat pointers and hence they will never be equal, meaning the wrapper os.FileInfo interface values will never be equal, meaning indexing the map with these FileInfo values will never yield any results.

\$\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.