3
\$\begingroup\$

I originally asked this question on StackOverflow. Reproduced here by recommendation.

I've got a number of Python scripts that I'm considering converting to Go. I'm seeking confirmation that I'm using Go in essentially the way it was meant to be used. Full code is available here: https://gist.github.com/anonymous/7521526

So the primary question is: Am I writing what is considered "good Go" ?

Python function #1: Look at the files in a folder and extract the latest encoded date in the filenames.

def retrieveLatestDateSuffix(target):
    return max(map("".join, filter(lambda x: x, 
        map(lambda x: re.findall(r"_([0-9]{8})\.txt$", x), 
            os.listdir(target)))))

My translation for #1:

var rldsPat = regexp.MustCompile(`_([0-9]{8})\.txt$`)
func retrieveLatestDateSuffix(target string) (string, error) {
    if fis, e := ioutil.ReadDir(target); e == nil {
        t := ""
        for _, fi := range fis {
            if m := rldsPat.FindStringSubmatch(fi.Name()); m != nil {
                if m[1] > t { t = m[1] }
            }
        }
        return t, nil
    } else {
        return "", e
    }
}

Python function #2: Add a date suffix to each filename inside a folder that doesn't already have a suffix added. Printing the from/to for now instead of doing the rename.

def addDateSuffix(target, suffix):
    for fn in filter(lambda x: re.search(r"(?<!_[0-9]{8})\.txt$", x), os.listdir(target)):
        pref, ext = os.path.splitext(fn)
        gn = "{0}_{1}{2}".format(pref, suffix, ext)
        print(fn, " -> ", gn)

My translation for #2:

var adsPat1 = regexp.MustCompile(`\.txt$`)
var adsPat2 = regexp.MustCompile(`_[0-9]{8}\.txt$`)
func addDateSuffix(target string, suffix string) error {
    if fis, e := ioutil.ReadDir(target); e == nil {
        for _, fi := range fis {
            if m1 := adsPat1.FindStringSubmatch(fi.Name()); m1 != nil {
                if m2 := adsPat2.FindStringSubmatch(fi.Name()); m2 == nil {
                    ext := filepath.Ext(fi.Name())
                    gn := fmt.Sprintf("%s_%s%s", strings.TrimSuffix(fi.Name(), ext), suffix, ext)
                    fmt.Println(fi.Name(), "->", gn)
                }
            }
        }
        return nil
    } else {
        return e
    }
}
\$\endgroup\$
  • 3
    \$\begingroup\$ rldsPat, adsPat1, adsPat2` - wtfPat? Plse dnt try to sve lttrs n vribl nms :) \$\endgroup\$ – ChrisWue Nov 18 '13 at 8:07
  • 1
    \$\begingroup\$ Just saw another post of this and your comment on it. To your bigger question of whether you should be using Go here, agree with your "maybe not". I can't tell if this is part of an app or standalone, but at least the task of adding and reading date suffixes on filenames is a great fit for Python/Perl; much as I like Go, I don't see much advantage to using Go for this task. (On the other hand, an app that needed or wanted lower-level bit-slinging, more direct OS access, concurrency, etc. might fit with Go's strengths.) \$\endgroup\$ – user2714852 Nov 20 '13 at 6:55
5
\$\begingroup\$

Early return is better than nesting. For example:

var adsPat1 = regexp.MustCompile(`\.txt$`)
var adsPat2 = regexp.MustCompile(`_[0-9]{8}\.txt$`)
func addDateSuffix(target string, suffix string) error {
    fis, e := ioutil.ReadDir(target)
    if e != nil {
        return e
    }
    for _, fi := range fis {
        if m1 := adsPat1.FindStringSubmatch(fi.Name()); m1 == nil {
            continue
        }
        if m2 := adsPat2.FindStringSubmatch(fi.Name()); m2 != nil {
            continue
        }
        ext := filepath.Ext(fi.Name())
        gn := fmt.Sprintf("%s_%s%s", strings.TrimSuffix(fi.Name(), ext), suffix, ext)
        fmt.Println(fi.Name(), "->", gn)
    }
    return nil
}

Also see this post for more idiomatic go discussion: https://stackoverflow.com/questions/20028579/what-is-the-idiomatic-version-of-this-function

\$\endgroup\$
0
\$\begingroup\$

Here's a script with both functions. Changes I made:

  • strings.HasSuffix will do to test for /\.txt$/.
  • os.Opening the dir and calling Readdirnames is shorter.
  • dateSuffixPat.MatchString is all addDateSuffix needs.
  • I used named return values like Jared did.
  • dateSuffixPat is a more descriptive name.
  • I used a string slice to cut off .txt; unsure if I think it's better, but it's shorter.

You could make further changes, like changing the APIs of these funcs to make a directory that's already been os.Open'd, or even just have them accept the list of names from Readdirnames; then you avoid repeating the os calls and error checks.

package main

import (
    "fmt"
    "os"
    "regexp"
    "strings"
)

var dateSuffixPat = regexp.MustCompile(`_([0-9]{8})\.txt$`)

const ext = ".txt"

func addDateSuffix(dirname string, suffix string) (err error) {
    dir, err := os.Open(dirname)
    if err != nil {
        return
    }
    fns, err := dir.Readdirnames(0)
    if err != nil {
        return
    }
    for _, fn := range fns {
        if !strings.HasSuffix(fn, ext) || dateSuffixPat.MatchString(fn) {
            continue
        }
        datedFn := fmt.Sprintf("%s_%s%s", fn[:len(fn)-len(ext)], suffix, ext)
        fmt.Println(fn, "->", datedFn)
    }
    return
}

func retrieveLatestDateSuffix(dirname string) (latest string, err error) {
    dir, err := os.Open(dirname)
    if err != nil {
        return
    }
    fns, err := dir.Readdirnames(0)
    if err != nil {
        return
    }
    for _, fn := range fns {
        if m := dateSuffixPat.FindStringSubmatch(fn); m != nil {
            if m[1] > latest {
                latest = m[1]
            }
        }
    }
    return
}

const dir = "."

func main() {
    err := addDateSuffix(dir, "19920101")
    if err != nil {
        // you would rarely panic in real life, but toy script, fine
        panic(err)
    }
    latest, err := retrieveLatestDateSuffix(dir)
    if err != nil {
        panic(err)
    }
    fmt.Println(latest)
}
\$\endgroup\$
  • \$\begingroup\$ Unfortunately, Go is just going to be longer than python for a lot of stuff. You can do some cool stuff with it, though. :) \$\endgroup\$ – user2714852 Nov 20 '13 at 6:16
  • \$\begingroup\$ And, not to bug, but there are a couple related questions over on SO with answers with upvotes that you haven't accepted. Might be worth accepting or clarifying why you don't think the question's been answered, depending on what you think. (Conflict-of-interest disclosure--I answered one of them.) \$\endgroup\$ – user2714852 Nov 20 '13 at 6:17

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.