ククログ

株式会社クリアコード > ククログ > クリアなコードの作り方: 意図が伝わるコミットのしかた

クリアなコードの作り方: 意図が伝わるコミットのしかた

コミットメッセージの書き方ではコミットをわかりやすくするためには以下の2つの条件を満たす必要があると書きました。

コミットの内容が分かりやすく説明されていること
コミットの内容が小さくまとまっていること

このうち「コミットの内容が分かりやすく説明されていること」についてはすでに説明済みです。今回は「コミットの内容が小さくまとまっていること」について説明します。

めざすところ

単純にコミットの内容を小さくするだけではわかりやすくなりません。それでは、どのような基準で小さくすればよいのでしょうか。

よく言われることは1つのコミットには1つの小さな論理的にまとまった変更だけにする、というものです。たしかにこれは重要です。しかし、これだけを基準とすると、人によっては大きめなコミットになってしまいます。人それぞれで論理的なまとまりの大きさが異なるからです。

1つのコミットでどうすればよいかを考えるのではなく、一連のコミットでどうすればよいかを考えましょう。そうすれば、1つのコミットにどこまで含めればよいかを考えやすくなります。感覚的に言うと「コミットの流れを見ているだけでペアプログラミングしている気分になる」コミットが小さくまとまっているコミットです。ここをめざしてください。

これを支援するためにはどのような開発環境がよいのかについてはここでは省略します¹。

コミット単位の例

いくつか小さくまとまったコミットの具体例を紹介します。

インデントを直す

名前の変更やコードの移動などのリファクタリングをした後に変更したコードの周辺だけインデントが崩れることがあります。このようなときはインデントだけを直すコミットをします。

よいコミット:

diff --git a/lib/test/unit/pending.rb b/lib/test/unit/pending.rb
index 75cc8cb..75b1914 100644
--- a/lib/test/unit/pending.rb
+++ b/lib/test/unit/pending.rb
@@ -112,8 +112,8 @@ module Test
       def handle_pended_error(exception)
         return false unless exception.is_a?(PendedError)
         pending = Pending.new(name,
-                                filter_backtrace(exception.backtrace),
-                                exception.message)
+                              filter_backtrace(exception.backtrace),
+                              exception.message)
         add_pending(pending)
         true
       end

もし、まわりにtypoなどがあってもそれをこのコミットに含めてはいけません。ペアプログラミングをしているときのことを思い出してください。1度に1つの作業しかできませんよね。

また、複数のファイルや複数のクラスなど、変更が複数の塊にまたがる場合は別々のコミットにしましょう。ペアプログラミングをしているときは、インデントの修正でコードが壊れていないことを確認するために、それぞれの塊を修正するごとにテストを実行しますよね。

indentコマンドを使うなど、一括で機械的にインデントを直す場合は1つのコミットにまとめても構いません。ただし、そのときはコミットメッセージに実行したコマンドラインを残しておくとよいでしょう。

typoを直す

たくさんコードを書いているとtypoはよくあることです。typoを直すときは同じtypo毎にコミットをわけましょう。またコミットメッセージにどんなtypoを直したかも書いておくとdiffを見た人に親切です²。

よいコミット:

diff --git a/lib/test/unit/notification.rb b/lib/test/unit/notification.rb
index 48ba3f6..c9c89b6 100644
--- a/lib/test/unit/notification.rb
+++ b/lib/test/unit/notification.rb
@@ -79,12 +79,12 @@ module Test
     module NotificationHandler
       class << self
         def included(base)
-          base.exception_handler(:handle_Notified_error)
+          base.exception_handler(:handle_notified_error)
         end
       end

       private
-      def handle_Notified_error(exception)
+      def handle_notified_error(exception)
         return false unless exception.is_a?(NotifiedError)
         notification = Notification.new(name,
                                 filter_backtrace(exception.backtrace),

typoの修正コミットに他の変更を混ぜるのはやめましょう。以下はtypoの修正とエラーメッセージの修正を1度にコミットしている悪い例です。

悪いコミット:

diff --git a/Rakefile b/Rakefile
index e3e73cf..bfcbe04 100644
--- a/Rakefile
+++ b/Rakefile
@@ -292,7 +292,7 @@ namespace :release do
       empty_options << "OLD_RELEASE_DATE" if old_release_date.nil?

       unless empty_options.empty?
-        raise ArgumentError, "Specify option(s) of #{empty_options.join(",")}."
+        raise ArgumentError, "Specify option(s) of #{empty_options.join(", ")}."
       end

       indexes = ["doc/html/index.html", "doc/html/index.html.ja"]
@@ -302,7 +302,7 @@ namespace :release do
          [old_release_date, new_release_date]].each do |old, new|
           replaced_content = replaced_content.gsub(/#{Regexp.escape(old)}/, new)
           if /\./ =~ old
-            old_undnerscore = old.gsub(/\./, '-')
+            old_underscore = old.gsub(/\./, '-')
             new_underscore = new.gsub(/\./, '-')
             replaced_content =
               replaced_content.gsub(/#{Regexp.escape(old_underscore)}/,

最初のhunkはjoinの引数にスペースを追加しているだけでtypoの修正ではありません。もし、コミットメッセージに「Fix typos」などと書かれていれば最初のhunkにもtypoがあるのではないかと思ってしまうでしょう³。

名前をつける

マジックナンバーに名前をつけるときは1つのコミットで1つのマジックナンバーだけに名前をつけましょう。

以下は、C言語のプログラムの終了コードを0と-1というマジックナンバーから、EXIT_SUCCESSとEXIT_FAILUREという名前のついた値にするためのコミットです。もし、間違って0をEXIT_FAILUREに置き換えていても気づかないでしょう。

悪いコミット:

diff --git a/src/groonga.c b/src/groonga.c
index fca1755..d193d15 100644
--- a/src/groonga.c
+++ b/src/groonga.c
@@ -1938,10 +1938,10 @@ do_daemon(char *path)
     break;
   case -1:
     perror("fork");
-    return -1;
+    return EXIT_FAILURE;
   default:
     wait(NULL);
-    return 0;
+    return EXIT_SUCCESS;
   }
   if (pidfile_path) {
     pidfile = fopen(pidfile_path, "w");
@@ -1951,7 +1951,7 @@ do_daemon(char *path)
     break;
   case -1:
     perror("fork");
-    return -1;
+    return EXIT_FAILURE;
   default:
     if (!pidfile) {
       fprintf(stderr, "%d\n", pid);
@@ -1959,7 +1959,7 @@ do_daemon(char *path)
       fprintf(pidfile, "%d\n", pid);
       fclose(pidfile);
     }
-    _exit(0);
+    _exit(EXIT_SUCCESS);
   }
   {
     int null_fd = GRN_OPEN("/dev/null", O_RDWR, 0);
@@ -2587,7 +2587,7 @@ main(int argc, char **argv)
     line_editor_init(argc, argv);
   }
 #endif
-  if (grn_init()) { return -1; }
+  if (grn_init()) { return EXIT_FAILURE; }

   grn_set_default_encoding(enc);

しかし、以下のようにEXIT_SUCCESSへの置き換えとEXIT_FAILUREへの置き換えを別のコミットにしたらどうでしょうか。これなら間違って置き換えていても気づきやすいですね。ペアプログラミングをしているときでも、EXIT_SUCCESSへの置き換えとEXIT_FAILUREへの置き換えを同時にやっていると、ペアの人が間違いに気づきにくくなりますよね。

よいコミット1:

diff --git a/src/groonga.c b/src/groonga.c
index fca1755..2731006 100644
--- a/src/groonga.c
+++ b/src/groonga.c
@@ -1941,7 +1941,7 @@ do_daemon(char *path)
     return -1;
   default:
     wait(NULL);
-    return 0;
+    return EXIT_SUCCESS;
   }
   if (pidfile_path) {
     pidfile = fopen(pidfile_path, "w");
@@ -1959,7 +1959,7 @@ do_daemon(char *path)
       fprintf(pidfile, "%d\n", pid);
       fclose(pidfile);
     }
-    _exit(0);
+    _exit(EXIT_SUCCESS);
   }
   {
     int null_fd = GRN_OPEN("/dev/null", O_RDWR, 0);

よいコミット2:

diff --git a/src/groonga.c b/src/groonga.c
index 2731006..d193d15 100644
--- a/src/groonga.c
+++ b/src/groonga.c
@@ -1938,7 +1938,7 @@ do_daemon(char *path)
     break;
   case -1:
     perror("fork");
-    return -1;
+    return EXIT_FAILURE;
   default:
     wait(NULL);
     return EXIT_SUCCESS;
@@ -1951,7 +1951,7 @@ do_daemon(char *path)
     break;
   case -1:
     perror("fork");
-    return -1;
+    return EXIT_FAILURE;
   default:
     if (!pidfile) {
       fprintf(stderr, "%d\n", pid);
@@ -2587,7 +2587,7 @@ main(int argc, char **argv)
     line_editor_init(argc, argv);
   }
 #endif
-  if (grn_init()) { return -1; }
+  if (grn_init()) { return EXIT_FAILURE; }

   grn_set_default_encoding(enc);

モジュールの中に移動する

最初は単なるちょっとしたコードだったものが他のコードでも使いたくなるくらい便利なコードに育っていくことはよくあります。そのようなとき、ライブラリとして使えるようにモジュールに入れたりしますね。

例えば、以下のようなちょっとしたログ出力メソッドがあったとします。

def log(tag, message)
  puts("[#{tag}] #{message}")
end

これをそのまま他のコードでも使おうとすると、トップレベルにlogメソッドが定義されてしまい、行儀がよくありませんね。このようなときは以下のようにモジュールの中に入れたりします。

module Logger
  module_function
  def log(tag, message)
    puts("[#{tag}] #{message}")
  end
end

このときは以下のように2つのコミットにわけます。

まず、モジュールで囲みます。しかし、まだ元のメソッドはインデントしません。

よいコミット1:

diff --git a/logger.rb b/logger.rb
index 1c7c4f0..7a3ed06 100644
--- a/logger.rb
+++ b/logger.rb
@@ -1,3 +1,6 @@
+module Logger
+  module_function
 def log(tag, message)
   puts("[#{tag}] #{message}")
 end
+end

次にモジュールの中身をインデントします。

よいコミット2:

diff --git a/logger.rb b/logger.rb
index 7a3ed06..293a335 100644
--- a/logger.rb
+++ b/logger.rb
@@ -1,6 +1,6 @@
 module Logger
 module_function
-def log(tag, message)
-  puts("[#{tag}] #{message}")
-end
+  def log(tag, message)
+    puts("[#{tag}] #{message}")
+  end
 end

このように分けることで、たとえ一緒に同じ作業をしていなくても、一連のコミットを見るだけで何をしようとしているかが伝わります。

1つのコミットのことだけを考えていると同時にコミットしたくなりますが、一連のコミットを考えるとこのように表現することもできます。意図が伝わるコミットです。

まとめ

コミットの内容を小さくまとめるにはどうしたらよいかの指針とその具体例をいくつか紹介しました。

1つ1つのコミットの積み重ねでクリアなコードが作られていきます。もちろん、1つのコミットは大切にしますが、一連のコミットも大切にして、意図が伝わるコミットにしましょう。コミットを見ることで、チームのみんなにどのように開発しているかが伝わるようなコミットにしていきましょう。

なお、フリーソフトウェアの開発のように、世界中の様々な場所・様々な時間に開発が行われているような場合はこのような意図が伝わるコミットのしかたがより重要になります。信頼されるようなコミットを重ねていきましょう。

例えば「diff入りのコミットメールを送る」という方法があります。 ↩
余談ですが、typoを直すコミットメッセージ中でtypoすることはよくある話です。 ↩
「Fix a typo」なら最初のhunkにはないと思うかもしれません。しかし、「そしたら最初のhunkはなんだろう？」ということになるのでそれでもよくありません。 ↩