Conversation
|
dep 使ってる偉い…！！ |
vvakame
left a comment
vvakame
left a comment
There was a problem hiding this comment.
自力で作りきるのが厳しかった理由はご自身の中で明確でしょうか？
明確であれば後は練習するだけですが、そうでなければ誰かに相談してみるのも良い方法だと思います。
|
|
||
| func main() { | ||
|
|
||
| url := os.Args[1] |
There was a problem hiding this comment.
この後に TODO check args とあるので自覚されている気もしますが、どのような引数が与えられた(与えられなかった)場合でもpanicにならないようにしましょう。
|
|
||
| d, err := NewDownloader(url) | ||
| if err != nil { | ||
| fmt.Printf("\ndownload initialize error. %v", err) |
| return http.DefaultClient.Do(req) | ||
| } | ||
|
|
||
| func getFileName(resourceUrl string) string { |
There was a problem hiding this comment.
自力で文字列操作するよりも、既存のものが既に準備されていないか調べるようにしましょう。
go で path を操作したいので、go path と検索してみます。
すると、 https://golang.org/pkg/path/ が見つかり、準備されている関数を調べていくと https://golang.org/pkg/path/#Base が妥当そうだな、とわかります。
| return nil, err | ||
| } | ||
|
|
||
| size := uint(res.ContentLength) |
| return d, nil | ||
| } | ||
|
|
||
| func NewWorker(d *Downloader, size uint, i uint, split uint, url string) (*worker, error) { |
There was a problem hiding this comment.
この関数はpackage外部にexportするべきかどうか考えてみましょう(しなくてよさそう
| bytesToFinishReading = size | ||
| } | ||
| w := &worker{ | ||
| processId: i, |
There was a problem hiding this comment.
プロセスIDと言われると別のものを想像するので、単に id か index が妥当な名前かもしれません
| if err != nil { | ||
| return errors.Wrap(err, "failed to open part file") | ||
| } | ||
| io.Copy(outputFile, partFile) |
There was a problem hiding this comment.
io.Copy はerrを返すので、無視せずにちゃんとチェックするようにしましょう。
https://golang.org/pkg/io/#Copy
errを無視している場合、それを怒ってくれるツールがあった気がします( go vet だっけ…？
いちいち調べるのがめんどくさい場合、そういうツールの助けを得るのはいい考えです
| return errors.Wrap(err, "failed to open part file") | ||
| } | ||
| io.Copy(outputFile, partFile) | ||
| partFile.Close() |
There was a problem hiding this comment.
Openした後に defer partFile.Close() とすることで、Close漏れを防ぐことができます。
"正しく処理する" ことを努力するより、 "嫌でも正しくなってしまう" 習慣を身につけるほうが後々楽ができます。
|
@vvakame レビューどうもありがとうございます！ |
2 participants
自力で作りきるのが厳しく、他の方の実装を参考にしつつ最低限ダウンロードまではできるように実装しました。
キャンセルが発生した場合の実装はできていません。
引き続きブラッシュアップしますが、現状の状態で申請いたします。お願いします。