戻り値を適切に設定して、メソッドの呼び出し階層を浅くしよう
対象
- 戻り値の使用法に不慣れな初級プログラマー
- 戻り値やメソッド分割の指導方法に悩んでいる経験豊富なプログラマー
- 私の所属するチームのメンバー
はじめに
プログラミングの初心者は、コードを書く際に変更を繰り返し過ぎてこじれてしまうことがあります。すでに組まれたコードに機能を追加したり、変更を加えたりすることで、コードが複雑化してしまいます。これは、プログラミングの初心者だけでなく、既存のコードに変更を加えることで、経験豊富なプログラマーにも起こり得ることです。
先日、私はそのように複雑化したコードのちょっとした見落としをしたため、利用部門とユーザーに迷惑をかけてしまいました。重大な不具合とまではいかなかったのですが、それによって混乱が生じました。この問題は、関数の呼び出し階層が深くなることにより、コードの可読性が低下してしまったことが一因でした。
今回は、まず初めにコードが複雑化しやすい原因となるプログラムの変更例を挙げます。その上で、戻り値を適切に設定することで、コードを分かりやすくする方法を紹介します。これが経験豊富なプログラマーの皆様が初級者を指導する際にお使いいただければ幸いです。
初級者が理解しやすいように、シンプルな例を用いて説明を行います。コードは幅広く読まれるJavascriptで記述し、より多くの人が理解できるように、基本的な構文のみを使用します。
失敗譚なので絵文字はバナナです。
概要
- メソッド名がウソにならないようにしよう
- メソッドは戻り値を使って階層を低く抑えよう
- データの取得と編集と出力はメソッドを分離しよう
例題
データベースに登録されているデータFooについて通知するnotifyFooメソッドについて考えます。通知はSlackで行います。
1. 初期状態
それほど複雑ではないコードです。
// Fooについて通知する
function notifyFoo(key) {
// データ取得はメソッド化されていることが多い
const data = getFooFromDB(key)
// Slack送信
postFooSlack(data)
}
function postFooSlack(data) {
// メッセージの編集(実際はもっと複雑だとします)
const message = {
id: data.slack_id,
title: data.key + "が登録されました",
body: "詳しくは" + data.url + "をご確認ください"
}
// slackに送信(汎用メソッド)
postSlack(message)
}
階層を表すと以下のようになります。
- Fooについて通知する
- データ取得
- Slack送信
- メッセージの編集
- Slack送信(汎用メソッド)
2. メール通知機能の追加
ここにメール通知も追加することになりました。メール通知はSlack通知のメッセージを元に作ります。
ここで問題のある変更が行われます。メール通知で一部Slack送信用で使った編集結果を使いたいために、編集が行われたメソッドに追記してしまいます。差分が追記であるため元々のコードを知っているメンバーには良いコードに見えます。(実際どういう理由でこうなったのかは知りません。)
// Fooについて通知する
function notifyFoo(key) {
// データ取得はメソッド化されていることが多い
const data = getFooFromDB(key)
// Slack送信
postFooSlack(data)
}
function postFooSlack(data) {
// メッセージの編集
const message = {
id: data.slack_id,
title: data.key + "が登録されました",
body: "詳しくは" + data.url + "をご確認ください"
}
// Slack送信(汎用メソッド)
postSlack(message)
// メール送信(ここ以下が追加)
postFooMail(data, message)
}
function postFooMail(data, message) {
// メッセージ作成(実際はもっと複雑だとします)
const mailMessage = {
address: data.email,
title: message.title,
body: data.Name + "様\n" + message.body
}
// メール送信(汎用メソッド)
postMail(mailMessage)
}
これも、番号付きの階層をで表すと以下のようになります。
- Fooについて通知する
- データ取得
- Slack送信
- メッセージの編集
- Slack送信(汎用メソッド)
- (追加)メール送信
- (追加)メッセージ作成
- (追加)メール送信(汎用メソッド)
かなり階層が深くなっています。
3. どうあるべきだったか
先程の例のような状態でわたしは、 postFooSlack
メソッド内に postMail
が存在することに気づきませんでした。
この原因は、メソッド名に反映されていない処理が含まれていたことにあります。読む側の私の責任もありますが、同時に書く側の我々の責任でもあります。勘違いのしないコードにする必要があります。
下記ですが、戻り値を利用することで、階層を浅く保ちます。
// Fooについて通知する
function notifyFoo() {
// データ取得
const data = getFooFromDB()
// slackメッセージ編集
const slackMessage = createFooSlackMessage(data)
// slack送信(汎用メソッド)
postSlack(slackMessage)
// mailメッセージ編集
const mailMessage = createFooMailMessage(data, slackMessage)
// mail送信(汎用メソッド)
postMail(mailMessage)
}
function createFooSlackMessage(data) {
// メッセージの編集
return {
slack_id: data.slack_id,
title: data.key + "が登録されました",
body: "詳しくは" + data.url + "をご確認ください"
}
}
function createFooMailMessage(data, slackMessage) {
// メッセージの編集
return {
address: data.email,
title: slackMessage.title,
body: data.name + "様\n" + slackMessage.body
}
}
メッセージ編集をメソッド化することは、「必要性が乏しい」と感じる方もいるかもしれませんが、 notifyFoo
のメソッドの処理の粒度を小さくすると目次のようになり読みやすさが増します。
また、関数を分割することで、テストをより詳細な単位で書くことが可能となります。特に、編集のメソッドが複雑だと予想される場合、テストが書けることは重要です。
階層構造は次のようになります。
- Fooについて通知する
- データ取得
- Slackメッセージ編集
- Slackメッセージの編集
- Slack送信(汎用メソッド)
- Mailメッセージ編集
- Mailメッセージの編集
- Mail送信(汎用メソッド)
機能追加しても、階層が深くならないので、読みやすさが保たれます。(ちょっと長くなっているのでさらに分割が必要かもしれませんね。)
メソッド分割に慣れていない場合は、データの「取得」「編集」「出力」の3つのフェーズごとにメソッドを分割することから始めてみてください。
余談
今回のコードはまだリファクタリングできますが、今回のテーマと合わないので割愛します。
次のようなことが可能だと考えています。
- 引数の情報量を減らす
- 直行したメソッドの分割
おわりに
今回は、メソッドの呼び出し階層が深くなることの問題点とその解決方法について説明しました。内容が理解しやすくなっていることを願っています。
このケースでうまくいくケースもあれば、逆のほうが良いケース、または違う意見などあると思います。有効な場合において説明するときの資料として使っていただければ幸いです。
感想やご意見などありましたら、お知らせください。
自己紹介
メディアエンジンの岩元(github)と申します。新卒から10年程度はメーカーの社内SEでCOBOLや色々な言語で書きつつ社内のいろいろなシステムに関わり、色々回り道をしてメディアエンジンにジョインしました。なかなかバグを作ることの才能に恵まれているため、修正しやすいコードの書き方を覚えました。
Discussion