4
\$\begingroup\$

This is a self-teaching implementation to get files for a given directory in order to simplify os.Walk in Go (avoid to pass a func for recursively walking across the files and directories).

I'm pretty new to the language and feel there is already something wrong with the way the structure ""implements"" the interface. I feel that it should not be by value but rather by reference in order to avoid redundant copies, should it?

package main

import (
    "os"
    "fmt"
    "path/filepath"
    "time"
)

type FileDetail interface {
    Path() string
    Info() os.FileInfo
}

type fileDetail struct{
    path string
    info os.FileInfo
}

func (detail fileDetail) Path() string {
    return detail.path
}


func (detail *ileDetail) Info() os.FileInfo {
    return detail.info
}

func GetFiles(path string) []FileDetail {
    var details []FileDetail

    walkFunc := func (filePath string, fileInfo os.FileInfo, err error) error {
        if err != nil {
            return err
        }
        if !fileInfo.IsDir() {
            var detail FileDetail = fileDetail{path: filePath, info: fileInfo}
            details = append(details, detail)
        }
        return nil
    }

    filepath.Walk(path, walkFunc)

    return details
}

func GetDirectories(path string) []FileDetail {
    var details []FileDetail

    walkFunc := func (filePath string, fileInfo os.FileInfo, err error) error {
        if err != nil {
            return err
        }
        if fileInfo.IsDir() {
            var detail FileDetail = fileDetail{path: filePath, info: fileInfo}
            details = append(details, detail)
        }
        return nil
    }

    filepath.Walk(path, walkFunc)

    return details
}


func main() {
    sandBoxDirectory := "F:/Perforce/eperret/XLC/R6Code/framework/source/scimitar/onlinemodule/Sandbox"

    time.Now()
    files := GetFiles(sandBoxDirectory)

    for index, file := range files {
        fileInfo := file.Info()
        fmt.Println(index, file.Path())
        fmt.Println(index, fileInfo.Name(), fileInfo.Size())
    }
}
\$\endgroup\$
3
\$\begingroup\$

Let's compare your two methods GetFiles and GetDirectories:

func GetFiles(path string) []FileDetail {
    var details []FileDetail

    walkFunc := func (filePath string, fileInfo os.FileInfo, err error) error {
        if err != nil {
            return err
        }
        if !fileInfo.IsDir() {
            var detail FileDetail = fileDetail{path: filePath, info: fileInfo}
            details = append(details, detail)
        }
        return nil
    }

    filepath.Walk(path, walkFunc)

    return details
}

func GetDirectories(path string) []FileDetail {
    var details []FileDetail

    walkFunc := func (filePath string, fileInfo os.FileInfo, err error) error {
        if err != nil {
            return err
        }
        if fileInfo.IsDir() {
            var detail FileDetail = fileDetail{path: filePath, info: fileInfo}
            details = append(details, detail)
        }
        return nil
    }

    filepath.Walk(path, walkFunc)

    return details
}

This is a classic case where copy&paste code is a problem. How can this be simplified? In Go, a good use-case will be a "file test" function that returns true if a file passes a test. Consider the "general purpose" private function that takes a "test" function as an input:

func filterFiles(path string, test func(os.FileInfo) bool) []FileDetail {
    var details []FileDetail
    walkFunc := func (filePath string, fileInfo os.FileInfo, err error) error {
        if err != nil {
            return err
        }
        if test(fileInfo) {
            var detail FileDetail = fileDetail{path: filePath, info: fileInfo}
            details = append(details, detail)
        }
        return nil
    }
    filepath.Walk(path, walkFunc)
    return details
}

Now, you can reuse that in your directories and files lists with:

func GetFiles(path string) []FileDetail {
    return filterFiles(path, func(file os.FileInfo) bool {
        return !file.IsDir()
    })
}

func GetDirectories(path string) []FileDetail {
    return filterFiles(path, func(file os.FileInfo) bool {
        return file.IsDir()
    })
}

Now you can also easily add interesting checks on file names, permissions, etc.

I realize that the interface FileDetail is there to teach you how they work, but in this instance, the interface is overkill, and I'd just keep things as a struct and move on.

As for the implementation of the interface, it's not horrible. Your concerns about the pointers is not really valid since all the data is small, and static.

It's OK in Go default to real data structures where the data is copied around, the memory cost is considered trivial.

|improve this answer|||||
\$\endgroup\$
  • 1
    \$\begingroup\$ Thanks, I was actually aware of the copy and paste thing, I thought about creating another method that would take another func (a bit like what you actually drafted above). About the struct part without the interface my only concern is that I wanted to keep it fully readonly, can I actually make it without leveraging an interface in Go? Also is it not cleaner to work on a pointer rather dealing with copies? \$\endgroup\$ – Kerry Perret Apr 12 '17 at 19:05

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.