\$\begingroup\$
\$\endgroup\$
I'm currently writing a little tool to get into Go. As I'm not familiar with the language I'm especially looking for
- Conventional go stuff.
- utility.go feels wrong.Should I wrap the client and email/password into a struct in utility? What would you advice.
Every critique on anything is very well welcome!
Small info:
- 2 Files
- main.go and utility.go
- utility.go has it's own package
main.go
package main
import (
w "./webactions"
"flag"
"fmt"
"net/http"
"net/http/cookiejar"
"os"
)
func main() {
if len(os.Args) < 3 {
fmt.Println("Not enough arguments provided")
return
}
format := flag.String("dl", "", "Set download format")
flag.Parse()
cookie, _ := cookiejar.New(nil)
client := &http.Client{Jar: cookie}
if w.Login(client, flag.Args()[0], flag.Args()[1]) {
fmt.Println("Loggin sucessfull")
if *format != "" {
if w.DownloadBooks(client, *format) {
fmt.Println("Downloaded all new Books as", *format)
} else {
fmt.Println("Failed to download any books")
}
}
if w.Logout(client) {
fmt.Println("Logout sucessfull")
}
}
}
utility.go
package webactions
type book struct {
nid string
title string
format string
downloadURL string
}
/*
Utility function for this file
*/
func getHTMLBytes(client *http.Client, urlString string) []byte {
rsp, err := client.Get(urlString)
if err != nil {
panic(err)
return nil
}
defer rsp.Body.Close()
htmlBytes, err := ioutil.ReadAll(rsp.Body)
if err != nil {
panic(err)
return nil
}
return htmlBytes
}
func Login(client *http.Client, email string, password string) bool {
form := url.Values{
"op": {"Login"},
"email": {email},
"password": {password},
"form_id": {"user_login_form"},
}
rsp, err := client.PostForm(requestUrl, form)
defer rsp.Body.Close()
if err != nil {
panic(err)
return false
}
return rsp.Request.URL.String() == requestUrl+"index"
}
func Logout(client *http.Client) bool {
_, err := client.Get(requestUrl + "logout")
if err != nil {
panic(err)
return false
}
return true
}
func DownloadBooks(client *http.Client, format string) bool {
htmlBytes := getHTMLBytes(client, myBooksURL)
if htmlBytes == nil {
return false
}
var wg sync.WaitGroup
//Every book has a div like <div nid="" title="">
books := regexp.MustCompile("nid=\"[0-9]+\" title=\"[\\w:\\[\\]'\\-\\s]+\"")
//Create dir
os.MkdirAll("BookDownloads/"+format+"/", 0666)
for _, e := range books.FindAllString(string(htmlBytes), -1) {
title := regexp.MustCompile("[\\w:\\[\\]'\\-\\s]+\\[eBook\\]")
nid := regexp.MustCompile("[0-9]+")
wg.Add(1)
go downloadBookJob(
client,
&wg,
&book{
nid.FindString(e),
title.FindString(e),
format,
requestUrl + "ebook_download/" + nid.FindString(e) + "/" + format,
},
)
}
wg.Wait()
return true
}
func downloadBookJob(client *http.Client, wg *sync.WaitGroup, book *book) {
//out = *file
if out := prepareDownload(book); out != nil {
defer out.Close()
if !downloadBook(client, book.downloadURL, out) {
os.Remove(out.Name())
wg.Done()
return
}
fmt.Println("Downloaded:", book.title)
}
wg.Done()
}
func prepareDownload(book *book) *os.File {
reg := regexp.MustCompile("[/:;?*<>|]")
fileName := "BookDownloads/" + book.format + "/" + reg.ReplaceAllString(book.title, "_") + "." + book.format
out, err := os.OpenFile(fileName, os.O_RDWR|os.O_CREATE|os.O_EXCL, 0666)
if os.IsExist(err) {
return nil
}
return out
}
func downloadBook(client *http.Client, bookDownloadURL string, file *os.File) bool {
rsp, err := client.Get(bookDownloadURL)
defer rsp.Body.Close()
if err != nil {
fmt.Println(bookDownloadURL)
panic(err)
}
if rsp.Header.Get("Content-Length") == "0" {
return false
}
_, err = io.Copy(file, rsp.Body)
if err != nil {
fmt.Println("Error while downloading", bookDownloadURL, "-", err)
return false
}
return true
}
200_success
146k22 gold badges190 silver badges479 bronze badges
1 Answer 1
\$\begingroup\$
\$\endgroup\$
1
A few suggestions:
Relative Imports:
- From personal experience, relative imports are a pain to work with. There is a very good discussion around this here. In short, as posted by Andrew Gerrand, the only reason the go tool supports relative imports is to run the compiler tests, which predate the go tool and the concept of GOPATH.
Do not Panic:
- Return an appropriate error instead of panicking so that caller can handle the error gracefully. The idiomatic way of returning an error is the last return value. I would suggest reading effective go.
Always check errors:
- Almost all the functions return an error. Check those before proceeding. You have deferred close of response body before checking the error which can lead to a panic if the HTTP request fails.
answered Jun 7, 2017 at 12:39
-
\$\begingroup\$ This provides some decent feed back, but it doesn't explain why. Is it possible to add explanations? \$\endgroup\$2017年06月07日 13:03:23 +00:00Commented Jun 7, 2017 at 13:03
lang-golang