生き様065. 勝手にコードレビュー ～イノウーの憂鬱 (25)　の感想～

学習

» 2020/11/19

イノウーの憂鬱、面白いですよね！

毎週、一人のファンとしてリーベルGさんのPress Enter■を楽しく読んでいます。
今回のコラムは、その先週の回の内容についてです。
そこで行われたコードレビューに対して、いくつか思う所がありました。
なので、それをコラムにしてしまおうと思います。

既に新しい回が公開されており、周回遅れ感はありますが…
先週頑張って書いていたのですが、間に合わず公開を見送ったのです。
そこから1周間掛けて、頑張って書き上げました！

大分長くなりますが、お付き合いいただけると幸いです。

全体的な感想

登場人物の一人である、マギ情報システム開発の三ツ橋さんには、とても同情してしまいます。
僕も、マギ情報システム開発の提出したコードは、おおよそ酷いものだと感じます。

ですが、手放しで攻める気になれません。
その背景として「前例踏襲」や「中途半端な流行の真似」という行動が見えるからです。
これらの行動は、現代日本のビジネスシーンにおいて、多々見られる悪習です。
ですから、三津橋さんも、その悪習の犠牲者だと、同情的に見てしまうのです。

それでは、ポイントとなりそうな箇所をピックアップしてみましょう。
タイトルにもなっている「クラス」「例外」「ロガー」「型定義」にも重なりますね。

1. 変数名は分かりやすい名前を付ける（中途半端な流行吸収）
2. 名称は統一しよう（前例踏襲？）
3. システムに登場する全ての項目をクラス化（中途半端な流行吸収）
4. メソッドの結果を必ずbooleanで返す（前例踏襲）
5. 独自のLoggerを実装（前例踏襲）
6. DBの文字コードフィールドの設定（前例踏襲）

これら一つずつについて、順番に見ていきたいと思います。
ただこき下ろすだけではなく「こうすればもっと良くなる」も含めてお話しましょう。

あくまでも【2020年11月現在の白栁のやり方】ですけれども。

※※お断り※※ 　今回のコラムで取り扱う原典は、フィクションです。 　その為、フィクションを元にした仮定の話になります。 　原典が2020年11月時点での技術的な前提に則していない可能性もあります。 　そもそも、フィクションに「現実として」ツッコミを入れています。 　そういう、とても野暮な話であることをご承知下さい。

ケース１：変数名は分かりやすい名前を付ける

最近のトレンドでは「命名には、わかり易さ重視でかな漢字を使う」があります。
僕はこれ、あまり好きではありませんがね。
でも、テストコードのメソッド名等には、積極的に使っています。わかりやすいので。

そもそも、何故かな文字、所謂2バイト文字や、半角カナを変数名やクラス名等の命名に使うのを嫌っていたか、ご存知でしょうか？

それは、文字コードの違いにより、文字化けしやすいからです。
文字化けにより、入力不可能な文字に変換されてしまったらどうでしょう？
正しく処理を指定することができなくなります。
昔のコーディング規約では
「文字化けしやすい特定の文字はコメントでも避ける」
という指定があったそうです。
ローマ数字等の機種依存文字も同じですね。

コンパイラによっては、
「文字解釈により1バイトずつ分解して読み込む為、2バイト文字だと壊れる」
という話があったとかいう噂も、うっすら聞き覚えがあります。
これは、都市伝説、与太話程度に聴いておいて下さい。

もう一つ、変換の為に入力を切り替える手間を惜しんだから、というものもあります。
プログラミング言語の基本構文は、多くが半角英字ですからね。
そこからかな入力に切り替えるのが手間だ、と考える人も居るでしょう。
といいつつ、コメントは入力を切り替えて日本語で書いていたりもします。
そこまで惜しむなら、コメントも英語で…と言った人が過去にいました。

さて、文字コードの話に戻りましょう。
最近では、マルチバイト文字のUTF-8が主流です。
これにより「半角だから1バイト、全角だから2バイト」という括りは無くなります。
つまり「文字化けを警戒して全角文字を使わない」理由がなくなります。

であるならば、文字による処理の違いはないのですから、
「文字種に関係なく読みやすいものにしよう」というのは至極当然に思えます。

もしかしたら、数年後には「ぴえん」の絵文字が変数名に入っている時代がくるかもしれませんね！
※システムの都合により、絵文字が使えない為、文字での表現になります

ケース１処方：そこまでわかり易さに拘るなら、かな漢字で命名すればいい

ローマ字を使ったこのケースでは、一つだけ誉められる部分があります。
英語で命名した場合、内輪でしか通用しない、独特の省略文字が使われたでしょう。
【OODE→CD】や【Master → Mst】程度なら可愛いですね。
【Format → Fmt】【Count → Cnt】とかまで行くと、どうでしょう？

ケース２：名称は統一しよう

作中では、消費税の項目に対して３つのパターンが出てきます。 「syouhizei」「shouhizei」「syohizei」ですね。
三津橋さんは「それぞれ別のメソッドですから問題はない」と言い訳をされています。
当然ですが、そんな訳ありません。

例えばこの消費税に対して、根本的な修正hが入ったとしましょう。
２年前に軽減税率が導入されていますから、現実的にありえない話ではありません。
命名がバラバラであれば、修正箇所の見落とし、漏れが発生する可能性が高くなります。

もしかしたら、ドキュメントでしっかりと管理されているかもしれません。
なので、消費税に関する修正箇所は、どこのクラスの、何処のメソッドにある、と突き止められるかもしれませんね。
ですがこの方法では、ドキュメントと修正したコードを突合するまで、修正の漏れの確認ができません。
もちろん、この方法は必要です。
名前の統一により、突合前にコードの上だけで確認することができます。

確認出来る方法は、手軽かつ数が多い方が好ましい、とは言うまでもありません。

僕は、なるべくデータベースの項目名に準拠した命名にする様にしています。
これなら、システム内で統一が図りやすいからです。

ケース２処方：システム内での名称・命名は統一しよう

ここからは余談です。
「実装されたクラスが違う」という理由で名称が変わる、について考えてみます。
これは、実装担当が「個人の趣味で命名している」ということです。
このケースで多いのが、「実装コードの重複」です。
担当者ごとに、同じ処理を別々に書いている事が多いのです。
メンテナンスコスト増大だけでなく、バグの温床にもなりやすい悪習ですね。

ケース３：システムに登場する全ての項目をクラス化

「システムに存在する項目を全てクラスとして用意する」
こう言われると、「とんでもない！」という感想を持つ人は、少なくないでしょう。
イノウー君もそうでした。
数年前の僕も、同じ感想を持っていました。

ですが、今はこのやり方が「多くの場合で適切だ」と考えています。
大規模なシステム開発であれば、迷わずに三津橋さんと同じ方法を取るでしょう。

「値オブジェクト」もしくは「ValueObject」と呼ばれるものがあります。
これは、最近のオブジェクト指向の流行です。
僕は「ValueObject」の呼び方が好きなので、以後はこれに統一します。

【ドメイン駆動開発（DDD）】とセットで語られる事が多いValueObjectですが、
僕は分けて捉えています。

ValueObject は クラスによる型表現を突き詰めたもの

と捉えているからです。 ValueObjectの利点は幾つかあります。
その最も大きな点は「型により値の種類が決められる」ことです。 例えば、単価に注文数量を掛けて金額を算出するメソッドがあります。

int price = Price.Calc ( int unitPrice, int orderQuantity );

これをValueObjectを使用して記述すると

Price price = Price.Calc ( UnitPrice unitPrice, OrcerQuantity orderQuantity );

となります。
どちらの方が、引数の設定ミスが少なくなるでしょう？ 単純な掛け算なので、単価と注文数量が入れ替わっても計算上の問題なさそうです。
ですが、複雑な処理の引数となれば、設定ミスが減らせる効果は大きいはずです。

とはいえ、マギ情報システム開発のクラス定義は、改善が必要でしょう。
僕であれば、最低限、この様にクラス定義をします。

 // ※ JavaDocコメントについては省略する
 public class TorihikisakiMei {

     // 取引先名
     private String torihikisakiMei;

     // コンストラクタ
     public TorihikisakiMei(String torihikisakiMei) {
         this.torihikisakiMei = torihikisakiMei;
     }

     // 文字列として取得
     public toString() {
         return torihikisakiMei;
     }
 }

不要なgetter/setterを廃して、コンストラクタでのみ値を設定できるようにしました。
文字列が必要であれば、toStringメソッドで取り出せます。
これにより、オブジェクトとしてのカプセル化がより進んだでしょう。

できれば、ジェネリクスで型を指定できる基底クラスを用意しておきたいです。
実際の値はそちらで保持することにします。
そして、toStringメソッドや、必要になるであろう比較式・演算式を抽象定義することで、
多態性も上手く使える仕組みを用意したいですね。
当然、定義した型の値を取り出せる仕組みには、getterは使わないでしょう。

また、実際の型の定義には「関連する処理を集約できる」という利点があります。
例えば、先程示した Priceクラスでは、価格を計算する為の処理を持っています。
特定の文字変換がある様な項目、範囲が適正かチェックする必要がある項目など、
処理を集約することで、メンテナンス性が高くなる場面は多いでしょう。

ケース３処方：ValueObjectを適切に利用して、値を型で表現しよう

ケース４：メソッドの結果を必ずbooleanで返す

これは、僕もやっていた時期があります。
そして、未だに見ることが多いコードでもあります。

その昔、処理は「関数」と呼ばれ「与えられた引数によって、値を返す」が主流でした。
値を返さない処理を「サブルーチン」と呼び、「関数」と分けるケースもありました。
ですが、多くの場合で「処理の成否を取得できる様にする」為に、関数が好まれました。 エラー詳細を文字列で返すものもありました。
この場合、文字列が空であれば、正常となります。

Exceptionであれば、型と値により、エラーの種類や状態を保持することができます。
また、専用のエラー構文へジャンプしますので、そこで適切に処理をすれば良いのです。
逐一、処理が返した値を確認して、処理分岐をする必要がありません。
この処理分岐で、ネストが深くなっているケースは、結構見かけました。

ですが、Exceptionを嫌う人が一定層居るのも事実です。
その多くが「処理の出口は一箇所に限る」という考え方を持っています。
これは、Jump処理により、可読性やメンテナンス性が損なわれる、という意見です。
ですが、Exception を return や goto 等のJump処理と同列に扱うのは、間違っています。

Exceptionによるエラー構文では、catch によりエラーが処理されます。
実際には、処理はJampしていますが、出口は１つです。
catch句の後に、finally句の処理を実行することからも、明らかです。
処理でエラーが発生した場合、if文でその後の処理を回避することと、実質的な動きは変わりません。
むしろ、try～cath や Exception を利用せずに if文のネストを深める方が、可読性やメンテナンス性を下げています。

ケース４処方：Exceptionはたくさんの情報を持たせることができる。 try～catch構文と合わせて活用しよう！

少しだけマギ情報システム開発を擁護すると、booleanで返しているだけ「マシ」です。
文字列を返す例については、前述しましたが、よりめんどくさいケースがあるからです。
それは、intのステータスを返すケースです。
そのステータスの定義は、システムによりバラバラです。
0が正常だったり、1が正常だったり。-1が異常、9が異常。3は異常があったが後続処理に影響はない など。 酷いと、処理ごとに違っていたりします。

ケース５：独自のLoggerを実装

僕は、独自のLoggerを実装すること自体は、必ずしも悪いことではないと考えます。

僕が今主に関わっているシステムでは、独自Loggerを実装しているからです。
ただの自己保身で言っている訳ではありません。
色々な検討を重ねた結果、欲しい情報を効率よくログに残す為には、独自のLoggerである必要があった、ということなのです。
守秘義務で詳細は話せません。開発運用一体が前提となっている、特殊なケースです。
「ログの種類により、ログの出力場所を変える」という仕組みも入っています。
個人情報関連のログが、誰でもアクセス出来る所にあったら、問題ですよね。

しかしさて。
マギ情報システム開発が今回採用しているmagiLoggerは、その様な検討を重ねた末での選択でしょうか？
そうではないのだろう、ということは、イノウーと三津橋さんのやり取りから感じられます。

全体的な印象ですが、マギ情報システム開発の独自ライブラリ郡は「開発効率を重視したもの」ではなさそうです。
少し悪意のある見解ですが「ブラックボックス化して、相手を煙に巻く為のライブラリ」に見えます。

magiLoggerで出力されたログは、
「マギ情報システム開発のメンバーは読みやすい（かもしれない）が、開発者以外にはとても読み辛い」
という可能性があります。括弧内がポイントです。
ワンチャン、普通にタイムスタンプとテキストを垂れ流すだけの仕組みかも知れませんね。
ログレベル指定の機能はあるようですが…。

トラブルが発生した場合は、別途高額な出張費用を払ってマギ情報システム開発の開発者を呼ぶ必要がある。
これはボロい商売ですね！
僕は、ユーザー側のシステム部門があるなら、ログでのトラブル切り分け程度は、任せたいのですが。
「ログを見る」だけで一々呼び出されていたら、堪らない！と考えるからです。
ユーザー側の管理者に、ある程度ログの確認方法を教えて、不明点だけ別途対応する方が楽です。
そしてこれが一番大事なのですが責任を共有できます。
しかし世の中、同じ様に考える人ばかりではないのでしょうね。勉強になります。

24時間365日、ログのサポートをしてくれるなら（有料でも）。
とてもサポートの手厚い会社だな、と感じます。

ケース５処方：ケース５処方：ログは「誰が・なんの為」を考えて実装する。責任の分担。

ケース６：DBの文字コードフィールドの設定

実は、ここだけは何が悪いのか、理解できませんでした。
僕のDB設計の知識が、大分古いことは自覚しています。
なので「英数字で決まった桁数のコードを格納する」フォールドであれば、DB上にcher型で定義します。

作品中では「実は桁数が決まっていなかった」ということで、問題となっています。
これが、桁数固定だった場合でもtext型で定義するべき、というなら、腑に落ちません。

char型は、桁数が固定れる、固定長データ型です。
つまり、決められた桁数のデータが入っている事がDBにより保証されるのです。
この1点だけでも、char型を使う利点があるでしょう。
また、varchar型やtext型等の可変長データ型は、キーやインデックスに利用するのに向かないはずです。
もしかして、この辺の事情が変わったのでしょうか？

「varchar型よりも、text型を使うべき」の部分には、納得できます。
特に、UTF-8の文字列であれば、制御コードの関係で、varchar型は適さないでしょう。
ただし「厳密に容量算定の必要がある」ケースでは、varchar型でサイズを決めるでしょう。
この辺りの事情も、ディスク容量の増加等で変わっているのかもしれませんね。

ケース６処方：「文字型の場合、char 型を使うメリットはほとんどない」理由を教えて下さい！

故人曰く「神は細部に宿る」

僕は、コードの1行1行に、意味があると考えています。
昔の職人は言いました。
「神は細部に宿る」と。

コードについても、同じです。
究極、プログラムなんて、仕様書の通りに動いて、ユーザーが望む動作をしてくれていれば、中身なんてどーでもいいものです。

しかし、本当にそれで言い訳がない、ということを我々は知っています。
もし、バグがあったとき。
もし、仕様変更があったとき。
より早く、より安全に、より確実に。対応を行うためには、コードの1行1行に拘る必要があります。

今回、やり玉に挙げられたマギ情報システム開発の三津橋さんの最大の失敗は、
汚いコードを書いたことではありません。
何故そのコードを書いたのか、よく考えなかったことです。

よく考えた上で間違っていたのなら、修正の余地があります。
よく言われる【宗派の違い】ってヤツならば、調整の余地が生まれます。

今回の結論

上で【2020年11月現在の白栁のやり方】と書いたのは、
僕もまだまだ勉強の途中だからです。
一年後、一ヶ月後、もしかしたら明日には、ここに書いたことをひっくり返している可能性があります。
特に、ケース６については、僕の知識が古いです。
なので、アップデートされることにより変わる可能性がとても高いです。

ITエンジニアは、常に学び続けなければいけません。
それは、必ずしも最新の流行技術を追いかけるだけではありません。

仕様やコードと向き合い、その中からより良い方法を探索し続ける。
その中にも、十分な学びはあります。

それよりも、その探索の中で、判断の根拠になる知識を集める必要がでるでしょう。
それこそが、本当に学び続けるという言葉の意味なのだと、考えています。

以上！

---

ミニコーナー：白栁の２ヶ月だけホンキダイエット！（最終回）

ここ１週間の体重推移報告です。

日	体重(kg)
11/12	90.1
11/13	88.3
11/14	88.9
11/15	90.1
11/16	90.1
11/17	88.9
11/18	88.4

最後のトレーニングが終わり、この企画も最終回を迎えました。
本来であれば、今回のコラムでまとめをする予定でした。
しかし、どうしても書きたい事ができたので、先送になりました。

改めてまとめた際に、総括やジムで計量した公式記録を載せたいと思います。

12月は、ダイエットのまとめ、婚活のまとめと、まとめ回が増えそうですね。

ITエンジニアの視点で、時事ニュースを5分間で紹介する動画を平日毎日公開してます。
「日々の生活の中にエンジニアリングがある」からこそ、
身近な時事ニュースから学ぶことが重要です。

#ほぼ日ITエンジニアニュース

パスワード付きZipをメール添付することがセキュリティ的に意味がない、とようやく周知され始めそうです。
今日公開された動画では、その話題を取り上げています。
これを切っ掛けに、日本のビジネスシーンでの情報セキュリティ意識が変わればいいな、と願っています。