故意の技術的負債から目を逸らさない
security-setting.jstwitterOAuth: [ body('twitterConsumerKey').if((value, { req }) => req.body.twitterConsumerKey).isString(), body('twitterConsumerSecret').if((value, { req }) => req.body.twitterConsumerSecret).isString(), body('isSameUsernameTreatedAsIdenticalUser').if((value, { req }) => req.body.isSameUsernameTreatedAsIdenticalUser).isBoolean(), ],
GROWI 各種認証機構設定のバリデーションで上記のように実装していました。
request body に該当する key が存在していた時のみ、チェックを行うというものです。
しかし、このコード非常に読みに難くなっています。
if((value, { req }) => req.body.twitterConsumerKey)
実装した本人が見ても少し違和感を覚えるコード... (過去の自分は他人である...)
このコードは express-validator の Docs external_link から引っ張ってきたものです。
Docs より引用
body('oldPassword')
// if the new password is provided...
.if((value, { req }) => req.body.newPassword)
書いてみて問題なく動いたので、特に可読性を意識せずに実装してしまいました。
セキュリティ画面の React 化1を早く終わらせたいと焦っていたのもあります。
プログラマが知るべき97というサイトでこのように書いてあります2
「正しくやる方法」と「手早くやる方法」があれば、後者のほうが魅力的に見えてしまうことはよくあります。
(中略)
次のイテレーションなどが修正のチャンスなのですが、実際にイテレーションが始まると、また新たな問題が起きてそちらに注力してしまい、結果、修正が不可能になってしまうことも多いのです。
このように先送りされていく修正作業のことを 「技術的負債(Technical debt)」 などと呼ぶことがあります。
終わらせたい一心で「手早くやる方法」をとってしまいました。
もちろん良いコードを書こうとして終わらないのは以ての外だとは思いますが、
少なくとも今回のように Docs の実装方法を丸パクリ するのではなく、そのライブラリでどのようなことが実現できるのか理解して実験することは必要でしょう。
今回は別の箇所で同じようなコードを実装していた時に指摘されたために気づき修正できましたが、これが 1年、2年と経つうちに手のうちようがない自体になったかもしれません。
技術的負債がいつ産まれるのかを痛感した出来事でした。
ちなみに修正後のコードはこのようになりました external_link。
security-setting.jstwitterOAuth: [ body('twitterConsumerKey').if(value=> value != null).isString(), body('twitterConsumerSecret').if(value=> value != null).isString(), body('isSameUsernameTreatedAsIdenticalUser').if(value=> value != null).isString(),
if(value=> value.exists()) でも動きますが、 null チェックであることを明示的に表すコードに落ち着きました。
もっと良い書き方がありましたら教えてください。