一個實例:為什么注釋是愚蠢的

jopen 9年前發布 | 11K 次閱讀 注釋

  • 原文地址(original source):http://simpleprogrammer.com/2015/04/13/why-comments-are-stupid-a-real-example/
  • 作者(author):https://推ter.com/jsonmez
  • </ul>


    當我寫一篇文章、或制作一個 油Tube 視頻,提及大部分情況下注釋不是必需的以及實際上弊大于利時,沒有什么比這更能激起宗教式討論了。

    在我讀《代碼大全》第二版時,我首先轉到這個討論的側面。

    在那本書,Steve McConnell 相當詳實整潔地給我闡明我在代碼放入如此多注釋的原因:

    • 我的命名沒有采用讓解釋型注釋成為不必要的方式。
    • 我的方法和函數過于龐大,而需要額外的解釋。
    • </ul>

      我寫代碼的方式發生了巨大的變化。

      我相信我過去做得還不錯,通過寫注釋解釋我的代碼、讓下一個開發人員更容易地理解它們,使我成為一名本分的程序員。

      但是,當我開始應用代碼大全里學到的東西,開始編寫經常比我之前寫的注釋還要整潔的代碼時,我意識到,我給那些接手我代碼的人所幫的忙,要比單純地寫注釋更有益處。我正在讓我的代碼更加整潔。

      當我讀 Uncle Bob 【注1】的《代碼整潔之道》一書時,我的觀點進一步加強了。

      Uncle Bob 不僅僅說我們應該避免注釋,還說了——并且演示了——注釋多半是怎樣在表達代碼意圖上失敗的。

      Bob 讓我意識到我仍然在深入學習注釋,我需要進一步提高我的命名,努力讓我的代碼無需外在幫助就能交流意圖。

      演示真實的移除注釋的例子

      這是我自己的個人發展——我不期望每個人都和我有同樣的經歷,或看到同樣的方式——但是,我覺得當我提起更多的會溝通的代碼(communicative code)、注釋通常應該避免時,還會有大量的無知要吐出來。

      過了一會兒,我覺得有必要支撐一下我要說的東東。

      對我而言,贊美整潔、會溝通代碼的價值是一回事,而像 Uncle Bob 所定義的“整潔”,相較于過度注釋的、同等“優秀”的代碼,清晰的代碼是多么地更容易被人理解和維護。

      起初我打算編造一些代碼例子向你演示這種情況。

      我打算寫一些代碼,有著不好的命名、充滿了注釋,然后向你演示我是怎樣重構代碼以除掉這些注釋并真正增加清晰程度。

      但是,據我經驗看,這是一個陷阱。

      很多人將吐槽、指責我設立了一個“稻草人”,它無法代表真實世界的代碼。

      幸運的是,我意識到一個絕佳機會正向我走來。

      重構來自 .NET 框架的真實代碼

      自從微軟決定開源整個 .NET 代碼庫后,我決定不去挑選一段不太糟糕的真實代碼示例,而要挑選可以被重構以消除代碼、且仍然讓代碼清晰的例子——據我看,要更清晰。

      我決定“挑選”的這段代碼位于這里

      我僅僅摘出這段代碼里的一個方法,SplitDirectoryFile,來闡明我的觀點。

      我不知道這段代碼的作者,甚至不認為這段代碼是糟糕的。我只是用它作為示例,請記住這一點。

      我挑選一個相當小的方法,這樣容易被理解,并適合本文,但是我的總體方法可以應用到更大型的代碼樣本里。

      在我重構之前,讓我們先看看這段代碼:

      internal static void SplitDirectoryFile(string path, out string directory, out string file)
      {
          directory = null;
          file = null;

      // assumes a validated full path
      if (path != null)
      {
          int length = path.Length;
          int rootLength = GetRootLength(path);
      
          // ignore a trailing slash
          if (length > rootLength && EndsInDirectorySeparator(path))
              length--;
      
          // find the pivot index between end of string and root
          for (int pivot = length - 1; pivot >= rootLength; pivot--)
          {
              if (IsDirectorySeparator(path[pivot]))
              {
                  directory = path.Substring(0, pivot);
                  file = path.Substring(pivot + 1, length - pivot - 1);
                  return;
              }
          }
      
          // no pivot, return just the trimmed directory
          directory = path.Substring(0, length);
      }
      return;
      

      }</pre>

      正如我上面說的,這段代碼還不錯。

      它相當清晰地表達了要實現的功能。

      注釋甚至更加明確。

      但是,我們能夠消除所有注釋并真正讓代碼更加易懂嗎?

      咱們從第一行注釋開始。它貌似非常無辜。

      // assumes a validated full path

      在這段代碼里,存在我們可以討論的方式嗎?

      用更好的參數名取代注釋

      如果我們把變量名從 path 改成 validatedFullPath,會怎樣呢?

      internal static void SplitDirectoryFile(string validatedFullPath, out string directory, out string file)
      {
          directory = null;
          file = null;

      if (validatedFullPath != null)
      {
          int length = validatedFullPath.Length;
          int rootLength = GetRootLength(validatedFullPath);
      
          // ignore a trailing slash
          if (length > rootLength && EndsInDirectorySeparator(validatedFullPath))
              length--;
      
          // find the pivot index between end of string and root
          for (int pivot = length - 1; pivot >= rootLength; pivot--)
          {
              if (IsDirectorySeparator(validatedFullPath[pivot]))
              {
                  directory = validatedFullPath.Substring(0, pivot);
                  file = validatedFullPath.Substring(pivot + 1, length - pivot - 1);
                  return;
              }
          }
      
          // no pivot, return just the trimmed directory
          directory = validatedFullPath.Substring(0, length);
      }
      
      return;
      

      }</pre>

      沒有太大差異——太瑣碎——但是,我們不但消除了一行注釋,我們也讓調用這個方法的人能夠根據變量名辨認出,它們不只是傳遞過來的 path,而是驗證過的 full path。

      重申,這貌似是個相當小的修改,但是,我相信它讓代碼更清晰了。

      稍微大點兒的改動……

      繼續,我們可以看到下面的注釋:

      // ignore a trailing slash

      現在,我們有了處理這種情況的一些方法。

      我們可以用一種方法簡單地取代注釋,因此有了:

      if (ShouldIgnoreTrailingSlash(length, rootLength, validatedFullPath)

      length–;</pre> <p>我們還可以創建簡單的布爾變量來取代注釋: </p>
      

      bool ShouldIgnoreTrailingSlash = length > rootLength && EndsInDirectorySeparator(validatedFullPath);

      if(ShouldIgnoreTrailingSlash)

      length–;</pre> <p>另外,或許更好的可能性就是下面的方法: </p>
      

      length = LengthWithoutTrailingSlash(length, rootLength, validatedFullPath);

      不過,我看到了一個機會,那就是改善整體閱讀性和代碼結構,并去掉注釋。

      這次該怎樣重構呢?

      public class DirectoryFileSplitter
      {
          private readonly string validatedFullPath;
          private int length;
          private int rootLength;

      public DirectoryFileSplitter(string validatedFullPath)
      {
          this.validatedFullPath = validatedFullPath;
      }
      
      public void Split(out string directory, out string file)
      {
          directory = null;
          file = null;
      
          if (validatedFullPath != null)
          {
              length = validatedFullPath.Length;
              rootLength = GetRootLength(validatedFullPath);
      
              IgnoreTrailingSlash();
      
              // find the pivot index between end of string and root
              for (int pivot = length - 1; pivot >= rootLength; pivot--)
              {
                  if (IsDirectorySeparator(validatedFullPath[pivot]))
                  {
                      directory = validatedFullPath.Substring(0, pivot);
                      file = validatedFullPath.Substring(pivot + 1, length - pivot - 1);
                      return;
                  }
              }
      
              // no pivot, return just the trimmed directory
              directory = validatedFullPath.Substring(0, length);
          }
      
          return;
      }
      
      private void IgnoreTrailingSlash()
      {
          if (length > rootLength && EndsInDirectorySeparator(validatedFullPath))
              length--;
      }
      

      }</pre>

      或許做得過度了,但是我認為把原來的靜態方法放到一個實際的類里,要清晰很多。

      然后我們可以非常清晰地說明該類應該做什么,并利用類的狀態來簡化代碼。

      盡量整潔地移除代碼,導致我重構了這個方法本身,因為我發現,表達這個意圖的最清晰方式就是將其封裝。

      現在,你或許認同了這種特定的重構——還不錯——但是我確信,你仍然想看到我們是怎樣通過更清晰地表達代碼意圖來輕松地去除注釋的。

      現在容易了

      去除剩下的注釋就有些瑣碎了:

      // find the pivot index between end of string and root

      我們所有不得不要做的修改就是將其改成剛才提到的方法。

      public class DirectoryFileSplitter
      {
          private readonly string validatedFullPath;
          private int length;
          private int rootLength;
          private bool pivotFound;
          public string Directory { get; set; }
          public string File { get; set; }

      public DirectoryFileSplitter(string validatedFullPath)
      {
          this.validatedFullPath = validatedFullPath;
          length = validatedFullPath.Length;
          rootLength = GetRootLength(validatedFullPath);
      }
      
      public void Split()
      {
          if (validatedFullPath != null)
          {
              IgnoreTrailingSlash();
      
              FindPivotIndexBetweenEndOfStringAndRoot();
      
              // no pivot, return just the trimmed directory
              if(!pivotFound)
                  Directory = validatedFullPath.Substring(0, length);
          }
      
          return;
      }
      
      private void FindPivotIndexBetweenEndOfStringAndRoot()
      {
          for (int pivot = length - 1; pivot >= rootLength; pivot--)
          {
              if (IsDirectorySeparator(validatedFullPath[pivot]))
              {
                  Directory = validatedFullPath.Substring(0, pivot);
                  File = validatedFullPath.Substring(pivot + 1, length - pivot - 1);
                  pivotFound = true;
              }
          }
      }
      
      private void IgnoreTrailingSlash()
      {
          if (length > rootLength && EndsInDirectorySeparator(validatedFullPath))
              length--;
      }
      

      }</pre>

      我仍然忍不住繼續修改這個結構。

      我把 DirectoryFile 移到了類的屬性,而不是方法的參數。

      但是,這沒有移除注釋和用方法取代它重要,然而在邏輯之上提供了一種抽象。

      我還把找到 pivot 的方式挪到了變量里,因此我們可以顯式地使用其狀態,而不是依靠之前的 return。(甚至都不用一行注釋來解釋)

      現在意圖更加清晰了。如果我們找不到 pivot,就只返回 trim 過的目錄。

      最終重構

      我們來到了最終的重構:

      public class DirectoryFileSplitter
      {
          private readonly string validatedFullPath;
          private int length;
          private int rootLength;
          private bool pivotFound;
          public string Directory { get; set; }
          public string File { get; set; }

      public DirectoryFileSplitter(string validatedFullPath)
      {
          this.validatedFullPath = validatedFullPath;
          length = validatedFullPath.Length;
          rootLength = GetRootLength(validatedFullPath);
      }
      
      public void Split()
      {
          if (validatedFullPath != null)
          {
              IgnoreTrailingSlash();
      
              FindPivotIndexBetweenEndOfStringAndRoot();
      
              if(!pivotFound)
                  TrimDirectory();
          }
      }
      
      private void TrimDirectory()
      {
          Directory = validatedFullPath.Substring(0, length);
      }
      
      private void FindPivotIndexBetweenEndOfStringAndRoot()
      {
          for (int pivot = length - 1; pivot >= rootLength; pivot--)
          {
              if (IsDirectorySeparator(validatedFullPath[pivot]))
              {
                  Directory = validatedFullPath.Substring(0, pivot);
                  File = validatedFullPath.Substring(pivot + 1, length - pivot - 1);
                  pivotFound = true;
              }
          }
      }
      
      private void IgnoreTrailingSlash()
      {
          if (length > rootLength && EndsInDirectorySeparator(validatedFullPath))
              length--;
      }
      

      }</pre>

      我們簡單地抽出了 trim 過的目錄,放到一個可以確切表明意義的方法里。

      現在,據我看,這段代碼已經非常清晰了,并且零注釋。

      我快速地瀏覽這段代碼,就理解了它的功能。

      這種結構和變量、方法的命名,比原來代碼里的注釋所要表達的意義,更加精確和清晰。

      比較和對比

      看下原來的代碼和這份重構的代碼,你認為哪一個版本更易于理解和維護。

      尤其要比較實現每種功能的主要方法:

      internal static void SplitDirectoryFile(string path, out string directory, out string file)
      {
          directory = null;
          file = null;

      // assumes a validated full path
      if (path != null)
      {
          int length = path.Length;
          int rootLength = GetRootLength(path);
      
          // ignore a trailing slash
          if (length > rootLength && EndsInDirectorySeparator(path))
              length--;
      
          // find the pivot index between end of string and root
          for (int pivot = length - 1; pivot >= rootLength; pivot--)
          {
              if (IsDirectorySeparator(path[pivot]))
              {
                  directory = path.Substring(0, pivot);
                  file = path.Substring(pivot + 1, length - pivot - 1);
                  return;
              }
          }
      
          // no pivot, return just the trimmed directory
          directory = path.Substring(0, length);
      }
      
      return;
      

      }</pre>

      public void Split()
      {
          if (validatedFullPath != null)
          {
              IgnoreTrailingSlash();

          FindPivotIndexBetweenEndOfStringAndRoot();
      
          if(!pivotFound)
              TrimDirectory();
      }
      

      }</pre>

      是的,我修改了代碼結構,讓其變成了類,我用屬性取代了輸出參數,但是我達到了類似的結果,消除注釋、用更有表現力的代碼和變量命名取代了注釋。(盡管如此,在特定情況下,我將承認輸出參數會增加代碼難度。)

      沒有注釋的代碼更容易維護

      然而,我要說的是,盡量用代碼表達的方式編寫代碼而非依靠注釋,使你以更好的方式構造代碼,這會因此產生更加內斂的類和更好的總體結構。

      不管你是否認為我這樣做得更好,你或許同意的一個地方就是,我肯定沒有使其變得更糟。

      如果你把有注釋的代碼重構為沒有注釋的代碼、且沒有損失清晰度,在我看來就是最大的勝利,因為代碼被更新了,注釋卻沒有。(至少它們經常沒有)

      記住,這是一些已經得體的代碼示例,這里的注釋不是非常不相關的。

      我見過的大部分注釋的代碼看起來根本不是這樣。

      代碼中的變量已經有著非常好的命名,注釋描述了邏輯要做的事情,而不是它是怎樣做到的。

      我見過的大部分帶注釋的代碼,使用注釋做為代碼的支撐,一點都不清晰。

      因此,在某種意義上說,我挑了一個糟糕的例子。

      我沒有挑選帶有糟糕注釋的丑陋代碼,并通過移除它們讓代碼更好——這應該也不難。

      不管怎么說,我靜候著憤怒評論家們的猛烈攻擊(沒有別的意思)。

      你是怎么看的?

      我已經完全說服你了嗎?

      你至少看到了編寫沒有注釋的代碼不是懶惰、而是真正有益處的——或者至少是一種偏好?