Java初心者です。(Java歴半年くらい)

こんなツールを作ってオープンソースで公開したのですが、自分の設計がこれでよいのか自信がありません。
設計上変なところがあったら指摘していただけないでしょうか?

https://github.com/DQNEO/S3ParallelUploader
フォルダを丸ごとAmazonS3にアップするアップローダです。
CLIで動きます。
一応テスト済みで、ちゃんと動きます。

(質問というよりコードレビュー依頼です)
よろしくお願いいたします。

回答の条件
  • 1人5回まで
  • 13歳以上
  • 登録:2012/11/08 01:45:19
  • 終了:2012/11/15 01:50:03
id:DQNEO

質問者から

DQNEO2012/11/08 01:47:54

カテゴリが「ウェブ制作」になってしもた orz

回答(4件)

id:oil999 No.1

oil999回答回数1728ベストアンサー獲得回数3202012/11/08 07:25:08

アップロードするファイル長の検査が無いような気がするのですが、間違っていたらごめんなさい。

id:DQNEO

そこはライブラリ側(AWS SDK)でやってくれているので大丈夫です。

2012/11/09 12:48:04
id:techmedia-think No.2

techmedia-think回答回数46ベストアンサー獲得回数132012/11/08 12:02:18

ポイント100pt

FileFinderの実装でメソッドやフィールドが全てstaticで定義されてますが、filesの内容の初期化が最初にされてるだけどなので、FileFinder#findを複数回呼び出すと前のfind結果も保持したまま要素が追加されることになりませんか?

id:DQNEO

おお、たしかに!
盲点でした。
FileFinderは外部から呼ばれることを想定してないので、パッケージの可視性をパッケージプライベート(でしたっけ?)に変更しようと思います。

指摘ありがとうございます。

2012/11/09 12:49:16
id:DQNEO

質問者から

DQNEO2012/11/09 12:50:49

質問文を編集しました。詳細はこちら

id:GRY No.3

Bright回答回数58ベストアンサー獲得回数22012/11/14 18:54:46

ポイント100pt

上の回答以外のことは問題なさそうです。

id:a-kuma3 No.4

a-kuma3回答回数4365ベストアンサー獲得回数18012012/11/14 23:37:16

ポイント100pt

クラス設計的な視点で。
ちっちゃいツールなんだから、細けーこと気にせんわ、ということなら、読み飛ばしてください。


com.amazonaws.services.s3 なパッケージの import があちこちに散らばってるのが気になります。
ぼくなら、Task か Uploader に、Amazon の都合を押し込めます。
どっち(もしくは、両方)に押し込めるかは、ちょっと悩みます。
Task は、Callable の実装が欲しいだけなので、Uploader の inner class にして、call メソッドでは Uploader のメソッドを呼ぶ、という形にするかも。

同じ理屈で、CLI は、あくまでもパラメータの解釈とチェックだけにとどめる。

UploadableFile は、java.io.File の気持ち悪さを引きずってます(個人的感想です)。
java.io.File が、OS が提供する「ファイル」の現身だとしたら、remove() とか renameTo() とかが、ものすごく気持ち悪く感じます。インスタンスとしての実体があるのに「消えろ」とかいうメッセージが。

	File f = ...;
	FileSystem fs = ...;
	fs.remove(f);

みたいな感じだったら、分かるんですけど。
なので、UploadableFile の upload や getS3Key メソッドが気持ち悪くって。
ぼくなら、Uploader で実装します。


「Amazon S3 の仕様が変わりました」とか「他のストレージサイトにも対応したい」というときに、何をどこまで変更しなきゃいけないのか、というところで効いてくるはずです。

他1件のコメントを見る
id:a-kuma3

ぼくなら、こうする、って雰囲気だけクラス図にしてみました。
FileFinder などのユーティリティなクラスは除外。
f:id:a-kuma3:20121115222910p:image

CLI から、AmazonS3Uploader って具体的なクラス名が見えてるじゃん、というのは、小物ツールなので。
Uploader の実装が増えたら、Factory を導入して、CLI は Uploader を知ってるだけ、というふうにする、という感じ。

2012/11/15 22:35:07
id:DQNEO

おお~~!!
めちゃ勉強になります!
確かに祖結合になってますね。

あとでリファクタリングしてみます。
ありがとうございます!

2012/11/16 18:18:44

コメントはまだありません

この質問への反応(ブックマークコメント)

「あの人に答えてほしい」「この質問はあの人が答えられそう」というときに、回答リクエストを送ってみてましょう。

これ以上回答リクエストを送信することはできません。制限について

絞り込み :
はてなココの「ともだち」を表示します。
回答リクエストを送信したユーザーはいません