id:pmoky氏のコードを読んでみた。

日常 プログラミング C#

まとめサイトにはってあるURLからZIPファイル落としてみた。
アップローダなるものをはじめて使ったんだが、使い勝手がわからずちと時間がかかったorz


どれから読んだらいいのかわからんのでとりあえず目に付いたものをかたっぱしから見て見よう。
ちなみに同梱されてるexeはDirectXのSDKいれないと動かないらしいので一切動かしてません。（ぉぃ
読んだファイルの名前と指摘をつらつら書いていく。


* そんな些細な突っ込みどうでもええやん？っていうものもあるけど、自戒のためにもとりあえず書いておく。


SGDevelop.cs

  • OpenFileDialogをインスタンスメンバーに持ってる理由って？？
    • 必要なときにインスタンシエーションすればいいし、そのほうがusingの恩恵にあずかれるんじゃないの？
    • OpenFileDialogを表示する場所なんてそんなに沢山ないし共通インスタンスを使いまわしてるわけでもないんだから処理を一箇所にまとめたほうが読みやすいんじゃない？
    • せめてSGDevelopのDisposeでDisposeしようよ
  • textOpen.Filterでわざわざ+で連結してる理由ってなんかある？
  • compileB_ClickとかDisposeせにゃならんもの(Processクラス)があるときはできるだけtry-finally使うべきでしょ。なんか例外あがったらプロセスリークになっちゃう。
  • errorCheckB_ClickでcscProcessを上書きnewしてるけどnullチェックせんでええの？
  • LoadDataのtry-usingでtry-try-finally-catchで無駄な気がするけど好みの問題だしまぁいいか
  • saveB_ClickでFileStream使ってるけどtry-finallyしてないよ


Util.cs

  • SGDevelop.csでPathクラス使ってるのにstaticメソッド内で使ってない理由って？
  • RemoveTemplateDirとRemoveTextDirってせめて引数の文字列長チェックくらいはチェックした方が・・・


Const.cs

  • スペルミス　Trandition -> Transition
  • Encodingって変数名にクラス名と同じ名前ってのは気持ち悪い。EncodingDisplayNameとかにした方が適切なんじゃない？
  • 値を書き換えないならpublic staticじゃなくてpublic constのがいいんじゃない？（クラス名もConstだし）


StartUpper.cs

  • delegateをクラス外で定義する理由がわからん（他のところでも使ってるのだろうか？）

今日は時間がとれなそうなのでここまで。
続きはまた明日。