loading...

Discussion on: Attempting to Learn Go - Listing Files By Extension

Collapse
detunized profile image
Dmitry Yakimenko

Steve, why do you sort inside the loop on every iteration?

for _, file := range dir {
    if !file.IsDir() {
        ...
        sort.Strings(m[ext[len(ext)-1]]) // <-- HERE
    }
}

And here's my take on it. You can sort by predicate. It's not exactly very efficient though, since the extension is recalculated every time. But come on, Go could be really annoying sometimes. Look at this verbosity:

type ByExt []string

func (a ByExt) Len() int           { return len(a) }
func (a ByExt) Swap(i, j int)      { a[i], a[j] = a[j], a[i] }
func (a ByExt) Less(i, j int) bool { return filepath.Ext(a[i]) < filepath.Ext(a[j]) }

...

func main() {
    ...
    files := []string{}
    for _, file := range dir {
        if !file.IsDir() {
            files = append(files, file.Name())
        }
    }
    sort.Sort(ByExt(files))
    ...
}

In Ruby that would be:

filenames.sort_by { |f| File.extname f }
Collapse
shindakun profile image
Steve Layton Author

Hah. You caught me! The sort should have been moved up into the print function(s) at the very least. It's a bad design decision - I put it there at first just for the sake of simplicity and never got around to cleaning it up. It doesn't really matter in a directory of a few files but would really impact performance in a larger directory. Something like the following might be fine, and still pretty simple to follow.

func plainList(m map[string][]string, v []string) {
    for _, value := range v {
        sort.Strings(m[value])
        for _, file := range m[value] {
            fmt.Println(file)
        }
    }
}

I think I may update the article to make sure it's called out for clarity.

Collapse
dirkolbrich profile image
Dirk Olbrich

You don't mind?

func plainlist(m map[string][]string, order string) string {
    // 1. get all keys of the map
    var keys []string
    for k := range m {
        keys = append(keys, k)
    }

    // 2. sort by order type
    switch order {
    case "desc", "Desc", "DESC":
        for ext := range m {
            sort.Sort(sort.Reverse(sort.StringSlice(m[ext])))
        }
        sort.Sort(sort.Reverse(sort.StringSlice(keys)))
    default:
        for ext := range m {
            sort.Strings(m[ext])
        }
        sort.Strings(keys)
    }

    // 3. build a concatenated string
    var list string
    for _, k := range keys {
        list = fmt.Sprintf("%v\n%v", list, m[k])
    }
    return list
}

use it with:

fmt.Println(plainlist(m, "asc"))
fmt.Println(plainlist(m, "desc"))
Collapse
dirkolbrich profile image
Dirk Olbrich

Nah, no need for the boilerplate to define a custom sort. This would do sorting the map:

var m = make(map[string][]string)
for _, file := range dir {
    if !file.IsDir() {
        fileName := file.Name()
        ext := strings.Split(fileName, ".")
        switch {
        case len(ext) > 1:
            m[ext[len(ext)-1]] = append(m[ext[len(ext)-1]], fileName)
        case len(ext) == 1:
            m["no-ext"] = append(m["no-ext"], fileName)
        }
    }
}
for ext := range m { sort.Strings(m[ext]) }
Collapse
dirkolbrich profile image
Dirk Olbrich

Edit: sorting the ˋ[]stringˋ within the ˋmap[string][]stringˋ. The map itself can't be sorted.

Thread Thread
detunized profile image
Dmitry Yakimenko

I don't have a map in my version. I sort an array by a predicate.

Thread Thread
dirkolbrich profile image
Dirk Olbrich

Yes, I have seen it. Your approach is different by just sorting a list of filenames. The orginal intent is to sort files by extension into different buckets.

Your use of filepath.Ext() is quit clever. Haven't thought of that.

Thread Thread
dirkolbrich profile image
Dirk Olbrich

This would make the example even shorter:

var m = make(map[string][]string)
for _, file := range dir {
    if !file.IsDir() {
        fileName := file.Name()
        ext := filepath.Ext(fileName)
        m[ext] = append(m[ext], fileName)
    }
}
for ext := range m { sort.Strings(m[ext]) }
Thread Thread
shindakun profile image
Steve Layton Author

@detunized @dirkolbrich

Thanks for the replies! filepath.Ext()! Didn't occur to me to try that. It goes to show that the standard library really is pretty complete.

Dirk, I like the for ext := range m { sort.Strings(m[ext]) } solution then I wouldn't need to have a separate sort in each "print" function, it's much clearer that way.