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
}
}
2 Answers 2
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
Here's a script with both functions. Changes I made:
strings.HasSuffix
will do to test for/\.txt$/
.os.Open
ing the dir and callingReaddirnames
is shorter.dateSuffixPat.MatchString
is alladdDateSuffix
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)
}
-
\$\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– user27148522013年11月20日 06:16:18 +00:00Commented Nov 20, 2013 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– user27148522013年11月20日 06:17:04 +00:00Commented Nov 20, 2013 at 6:17
rldsPat,
adsPat1,
adsPat2` -wtfPat
? Plse dnt try to sve lttrs n vribl nms :) \$\endgroup\$