作っているソフトのソースコードのレビューをしてもらった。
-
- いいね！1
ありさん@Web系かTesterで求職中
@arigayas
@d_toybox ニコニコ生放送の視聴者コメントを表示するソフトを作っているのでそう言われましたも・・・。(ボケつぶしだったらごめんなさい)
2013-07-14 09:05:34
なかのんの旅々(Masayuki Nakano)
@d_toybox
@arigayas でも、バグを探す時は、地道にコードを理解して脳内でエミュレーションした方がええよ。それで理解できないところは複雑に書きすぎてるからシンプルにした方が良い。
2013-07-14 09:10:37
ありさん@Web系かTesterで求職中
@arigayas
@d_toybox エラー処理(コメントが飛んだらその分「NGコメントです」と入れる)を書いているんですがそれのせいでかなり複雑になってしまった(*´д｀*) で今対応しているのは（恐らく特定の条件で）何故か最後のコメントが表示されずに終わってしまうんです(´・ω・`)
2013-07-14 09:15:51
なかのんの旅々(Masayuki Nakano)
@d_toybox
@arigayas まずはあのループ、ばらして、ループを見た目、一重しかないようにすれば、かなり読みやすくなるよ。ループ内でやってる処理をそれぞれメソッドにすれば、コメント書かなくても、メソッド名で何をやってるか分かるようになるし。
2013-07-14 09:17:25
ありさん@Web系かTesterで求職中
@arigayas
@arigayas @d_toybox 同時に扱う変数が3つか4つあって物によっては減ったり増えたりするので脳内でエミュレーションするのは無理です(；´д⊂）
2013-07-14 09:17:26
なかのんの旅々(Masayuki Nakano)
@d_toybox
@arigayas せやから、理解できへん状態がもう書き方悪いねん。小分けになってなくて、デカすぎるコードになってるから理解できへん。
2013-07-14 09:19:27
なかのんの旅々(Masayuki Nakano)
@d_toybox
@arigayas https://t.co/72y8BVo51G こんな感じで、やりたいこと単位で、でかいメソッドをばらしていくと、100行以上あったコードが、こんな風に、見通しが良くなる http://t.co/yUx0Y4Gahu
2013-07-14 09:23:36
ありさん@Web系かTesterで求職中
@arigayas
@d_toybox widget/windows/nsWindow.cpp のようにってことですか？
2013-07-14 09:26:03
なかのんの旅々(Masayuki Nakano)
@d_toybox
@arigayas そう。このバグ、nsWindow内のでかいメソッド達を、別のクラスに追い出してるバグで、その作業を20段階ぐらいに分割して、細かく移動させてる。
2013-07-14 09:27:25
なかのんの旅々(Masayuki Nakano)
@d_toybox
@arigayas メソッドを細かく分けておけば、変数もできるだけ狭い空間に閉じ込められるし、コード変更する時に、ごく狭い範囲にしか影響を与えないことを確実にしながら、変更できるのでメンテナンス性も上がる。
2013-07-14 09:28:46
なかのんの旅々(Masayuki Nakano)
@d_toybox
@arigayas 後から見ると、細かい単純作業をこなす大量のメソッドと、それをシンプルに呼び出すだけで、流れが分かりやすいメソッドの二種類に分かれてるはず。
2013-07-14 09:29:58
なかのんの旅々(Masayuki Nakano)
@d_toybox
@arigayas 実際、私が、何かのバグの準備でやってるリファクタリングって、細かい、発見されてなかったバグが何個か修正されてたりする。
2013-07-14 09:31:06
ありさん@Web系かTesterで求職中
@arigayas
現状そうですもん(；´д⊂） RT @d_toybox @arigayas それぐらい、でかくて煩雑になってるところには、バグが産まれてしまう。
2013-07-14 09:33:08
なかのんの旅々(Masayuki Nakano)
@d_toybox
@arigayas せやから、バラしていくというリファクタリングすんねん。そしたら自然と見つかる場合が多い。細かく分けたら、考慮漏れのバグが激減する。
2013-07-14 09:33:59
ありさん@Web系かTesterで求職中
@arigayas
@d_toybox 今までのソースコードを捨てる勇気が無かったけど、なかのさんの説明を受けていると作り直した方がいい気がしてきました∠( ゜ω゜)／
2013-07-14 09:35:51
なかのんの旅々(Masayuki Nakano)
@d_toybox
@arigayas 捨てるんやなくて、分離して整理するだけやね。そしたら見通し良くなって、簡単に頭でエミュレーションできるようになる。
2013-07-14 09:38:21
なかのんの旅々(Masayuki Nakano)
@d_toybox
@arigayas とにかく、よほど単純な場合を除いて、ループをネストしたらあかんよ。内側のループは別のメソッドに追い出した方が見やすい。
2013-07-14 09:39:55
ありさん@Web系かTesterで求職中
@arigayas
@d_toybox いやこのコード当初は「NGコメント」って入れるつもりはなかったので再設計した方がいい気がしたのと設定で切替えできたほうが良いと思ったので作り直そうかなと
2013-07-14 09:41:18
なかのんの旅々(Masayuki Nakano)
@d_toybox
@arigayas その入れる部分だけ、新しいメソッドで作って、それを元のコードから呼び出してるだけなら、元の処理はバグらんやろ?
2013-07-14 09:42:05
ありさん@Web系かTesterで求職中
@arigayas
@d_toybox NGコメントを入れる部分はループの外へ出しました。http://t.co/4CR4LvWYmi 恐らく1番目から複数個NGコメントになってその後にまたNGコメントがあると最後の行のコメント(番組終了コマンド)が表示する処理される前にループが終わってしまうんです
2013-07-14 09:50:46