« 2014年11月 | トップページ | 2015年1月 »

2014年12月の記事

クソコード撲滅ワーク!

12月の定例会で、「クソコード撲滅ワーク!」を開催しました!

Pict2674
以前の定例会で、アーキテクトとして活動することも多い三文字さんから、顧客先での勉強会として「ソースコード・レビュー勉強会」が行われた事に関して書きましたが、
クソコードは本当に頭にくる!」ということから、クソコードのサンプルを共有して叩き合ってみようじゃないか!?、という事になりました(あ、言語はPHPを選びました)。

で、三文字さんがネット上から見付けたモノ、仕掛中の案件内で見たパターンを再現したモノ、これまでの経験上から書き起こしたモノなど、十数点のクソコードをプロジェクターで映しながら共有しました(写真と説明が同期していません。申し訳ないです.....が、どれがどんな問題を抱えているか?、宜しければ一緒に考えてみて下さい!、ツッコミどころ満載ですよ!)。

※本番のwarning出力をオフにするコード
 結局、終盤になって気が付き、Warningの山になり、対処に大わらわになった。

Pict2675
※arrayGetDuplicate

 配列の重複値を返すメソッドだが、ネットに落ちていたコードをそのままもってきていた。
 バグを含んでいたコードだったが、結果として呼ばれていなかった点もお粗末。
 教訓:ネットで版権問題の無いソースコードを拾ってくる場合も、理解してから使うのが大前提!

※変数の空文字チェックを行うメソッドをわざわざ作っているパターン

Pict2677
※)「祈る」:
"何かあったら下記まで連絡してください。" というコメントがあった。
 実際、三文字さんは記載の有った番号に連絡する羽目に至った。
 メソッドとコメントが合っていない例なども結構ある!

※コンストラクタに全ての処理が入っているパターン。
 オブジェクト指向をまるで理解していないんじゃないのか?

Pict2678
※switch文で、defaultが最初に書かれている
パターン。
 普通、defaultは最後でしょ?(ローカル事情も有るのかも知れませんが...)

※Userクラス内に定義された削除フラグの名前がおかしい

Pict2680
※do {

      処理
 } while(false)

 "break;"をしたいが為のコーディング。
 分からなくもないんですが、どうにかならんもんか?

Pict2683

※switch文が入れ子になっている
パターン。
 業務的に書きたくなる場合も有るかも知れないが、リファクタリングすべきところ。

Pict2682
※空文字を定義して、変数に定義した空文字で初期化している
場合。
 気持ちは分からなくもないが、直接、空文字を設定すればよいのでは?

※if分のtrueのケースに、処理が何も書かれていないケース。
 逆のロジックを書けば良いだろう!?、というケースですが、極レアな条件だけを除きたい場合などには、書きたくなる場合も有りますね。

Pict2684
※staticメソッドだけのクラス

 業務処理に近い層のクラスで、この実装は無いでしょう。

Pict2687
※添付ファイルだから、変数名が$temp_file!?

 ダジャレか?、いや書いた本人は真面目だから困るパターンです。

Pict2688
※メソッド名やクラス名に、個人名称とか、全く機能と関係の無い名称を付ける

 趣味のコーディングじゃないんだから...。

Pict2686Pict2681

更に、三文字さんから短時間でコーディング可能そうなお題が出されて、2014年新人の速形さんが、リアルタイムでコーディングに臨みました!

1メソッドの実装だけだったので、特に問題無く(先輩達からの厳しいツッコミもあまり無く)完了できました!

それにしても、リアルタイム・コーディングは面白いですね!

以前から、デモベースで勉強会(定例会の中で)を行う際に、コードを変更しながら動作の違いを見て行くようなパターンは有りましたが、やっぱり「コードが見える・変化していく」過程を見ること自体が面白いです。

書く人の暗黙知の一端を垣間見ることもできますし。

結構、刺激的でもあります!

皆で楽しめるのは、何よりですね!!


★クリック1つでブログランキングにご協力戴けます! m(_ _)m

| | コメント (0) | トラックバック (0)

Androidアプリの抜き打ち?コードレビュー

この話も10月の定例会でのコンテンツなのですが、
ほぼレギュラーコンテンツ」として、「IT-Tips 又は 5分間スピーチ」というものがあります。

業界的なちょっとしたネタとか、或いは全く関係の無いこと(数回前にアップした「他業種での経験も大切だよ!、という話」とか)を制限時間内に収めるように話すとか。

要は、相手が自社内のメンバー達であっても、「人前で話すことに慣れる」(しかも時間制限付きで)、「何か話す話題を探しておく」(普段から(直前になって慌ててでも?)ネタは無いか?と、アンテナを働かせる)、「業務外で、こいつは何に関心を持っているんだろう?」(相互理解)などを目的としたコンテンツで、全社員・持ち回りで実施しています。

Pict2671
この会の時は、今年入社したばかりの速形さん

実はネタを考えていなかったという「こいつめっ!」状態だったのですが、試作中のAndroidアプリの試験的実装のコードレビューを「いきなりお願いしますっ!」という大胆な展開となりました

元々の時間枠が10分ですし、そもそもレビューの前提条件が全く整っていないという、本人以外の全員にとってもカウンターパンチ状態でした!

が、ともかくも見えている範囲(プロジェクターでソースコードの一部を投影しています)で、参加者たちから指摘や質疑・応答、アドバイスが、あれやこれやと出て来ます

言語は、普通にAndroid標準的なJavaです。

映された範囲では、幸いにも?「クソコード」との指摘は有りませんでした(「クソコード」は、前回の記事で話題にしましたね)。

参加メンバー達は、と言うかアスペアの技術者たち全員に言えることですが、実務上の役割(ロールの比率;大抵は複数のロールを担当するのが当然ですから)を別として、
とにかくモノ造り(コードを書くこと)が大好きです!

やっぱり人のコードを見るのは面白いよねー!
意見も様々に出るし、お互いの視点や考えを聞くこと自体も面白い!

...ってことも有って、「定例会の中でコードレビュー的なコンテンツやりたい!」ってことになりました。

定例会を企画・調整している立場としては、こんなふうに「こんなことやりたい!」がメンバー達から湧き出て来てくれるのが一番嬉しいです。
(まあ、単純に「自分がラクだ」ってのも確かではありますが)

やりたいこと、楽しい事を仕事としていけるのは素晴らしいことじゃないか!?と思います!


★クリック1つでブログランキングにご協力戴けます! m(_ _)m

| | コメント (0) | トラックバック (0)

« 2014年11月 | トップページ | 2015年1月 »