---
title: "寫程式的品味"
date: 2021-11-14T00:00:00+08:00
publishDate: 2021-11-14T14:48:53+08:00
lastmod: 2022-01-17T14:20:52+08:00
tags: ["Ruby","程式","心得","工程師","Legacy Code"]
toc: true
permalink: "https://blog.aotoki.me/posts/2021/11/14/the-programmer-taste/"
language: "zh-tw"
---


前幾天同事根據客戶的需求在實作功能時請我幫忙檢查設計上是否有問題，因為是一段遺留代碼（Legacy Code）因此調整起來也不是那麼輕鬆。

其實同事的修改方式讓我不太滿意所以給了建議去調整，這是很多新手工程師會遇到的狀況，工作很少有機會去看到高品質的程式碼，最後就只能依照有問題的遺留代碼繼續修改跟當作學習參考。

<!--more-->

## 自由的程式{#programming-is-free}

原本的功能是一段會將第三方服務的資訊以及資料庫中的資訊整合之後轉換成快取（Cache）的機制，這一次客戶希望加入更新過程中如果發生錯誤就取消更新的功能，同時要將出錯的資訊記錄起來。

原始的程式碼大概是這樣的

```ruby
class FirmwareRefresher
  attr_reader :refreshed_firmwares

  def initialize
    # ...
  end

  def perform
    @refreshed_firmwares = []
    @devices.each do |device|
      load_info(device)
    end
    update_firmwares(refreshed_firmwares)
  end

  def load_info(device)
    third_party_info = Vendor::Client.get(device.id)

    # ...

    refreshed_firmwares << firmware
  end
end
```

大多數時候新手工程師剛入門會遭遇的問題是不知道怎麼寫，脫離了課程、教科書的範例之後就不知道該怎麼實現一個功能，當工程師是很需要**創意**的。

上面這段程式可以正常運作，客戶也使用了好幾年也因此同事就像這樣將他擴充出來，加入了發生錯誤就不更新的機制。

```ruby
class FirmwareRefresher
  attr_reader :refreshed_firmwares

  def initialize
    # ...
  end

  def perform
    @refreshed_firmwares = []
    @errors = [] # New Line
    @devices.each do |device|
      load_info(device)
    end
    update_firmwares(refreshed_firmwares) if @errors.empty?  # New Line
  end

  def load_info(device)
    third_party_info = Vendor::Client.get(device.id)

    # ...

    refreshed_firmwares << firmware
  rescue Vendor::RequestError => e  # New Line
    @errors << e
    ErrorReporter.capture(e)
  end
end
```

像這樣的修改，實際上沒什麼問題也依舊可以正常運作，然而對熟悉 Ruby 這個語言的工程師來說，總是有那麼一點點的「違和」感。

同時，如果是一個熟悉物件導向語言的工程師，也還是會認為這樣的程式能夠再做出一些改進，對我來說這就是所謂的「品味」問題，因為我們可以有更簡潔的方式去實現這個功能。

## 停止的時機{#timing-to-stop}

首先，在例外（Exception）處理的機制上我們通常會要先思考的是「誰知道怎麼做」跟「何時該終止」這兩個問題。

這個問題客戶並沒有給很明確的需求，我們應該還要再進一步確認。不過單純就「發生錯誤就不更新」這一個前提來看，一旦更新出錯一次就可以直接停止。

也就是說，我們可以將程式碼加以修改調整處理的時機。

```ruby
class FirmwareRefresher
  attr_reader :refreshed_firmwares

  def initialize
    # ...
  end

  def perform
    @refreshed_firmwares = []
    @devices.each do |device|
      load_info(device)
    end
    update_firmwares(refreshed_firmwares)
  rescue Vendor::RequestError => e  # Changed
    ErrorReporter.capture(e)
  end

  def load_info(device)
    third_party_info = Vendor::Client.get(device.id)

    # ...

    refreshed_firmwares << firmware
  end
end
```

透過一個簡單的調整，我們就可以減少一些多餘的實作跟程式呼叫。除了能夠稍微加速程式運作之外，也讓實作變的相對的簡潔和清晰。

## 難以閱讀的結構{#unreadable}

然而，我們還是有著遺留代碼（Legacy Code）所帶來的問題，在原始版本中我們要去理解這個物件的運作會面臨幾個問題。

1. `#load_info` 的使用時機？
2. `refreshed_firmwares` 的內容？

以文章的範例來看，會發現要了解這段程式碼會需要完整的閱讀一次才會知道 `#load_info` 會用來產生 `refreshed_firmwares` 的內容。

同時，又會有更多的疑惑出現。像是 `refreshed_firmwares` 是公開的方法，所以會被其他物件存取嗎？在這個狀況下 `#load_info` 也是公開的，可以被單獨呼叫嗎？如果可以，那麼回傳值是處理到一半的 `refreshed_firmwares` 嗎？

很多時候我們覺得一個專案複雜，並不一定是能力不足，而是有些人在撰寫他的程式的時候把問題變複雜，或者沒有釐清問題就去實作這些機制而造成更多問題的出現。

## 了解語言特性{#knowing-language-features}

因為不清楚原始程式完整的時空背景，因此我們單純就對程式語言跟架構設計的面向去理解這段程式碼可以怎樣改進。

根據前面的問題，實際上 `#load_info` 和 `refreshed_firmwares` 都被設計為 `private`（私有）會比公開更好，也因此我們可以像這樣調整。

```ruby
class FirmwareRefresher
  def initialize
    # ...
  end

  def perform
    @refreshed_firmwares = []
    @devices.each do |device|
      load_info(device)
    end
    update_firmwares(refreshed_firmwares)
  rescue Vendor::RequestError => e
    ErrorReporter.capture(e)
  end

  private # New Line

  def load_info(device)
    third_party_info = Vendor::Client.get(device.id)

    # ...

    refreshed_firmwares << firmware
  end
end
```

因為 `@refreshed_firmwares` 不需要再繼續被公開，我們其實也不需要這個實體變數（Instance Variable）可以轉換為區域變數（Local Variable）

```ruby
class FirmwareRefresher
  def initialize
    # ...
  end

  def perform
    refreshed_firmwares = @devices.map { |device| load_info(device) } # Changed
    update_firmwares(refreshed_firmwares)
  rescue Vendor::RequestError => e
    ErrorReporter.capture(e)
  end

  private

  def load_info(device)
    third_party_info = Vendor::Client.get(device.id)

    # ...

    firmware # Changed
  end
end
```

像這樣，我們的程式碼又更乾淨了一些，只要直接閱讀 `#perform` 方法就可以理解到足夠的資訊，如果在未來要修改或者調整的時候也不需要知道 `#load_info` 是怎麼做的。

如果你對 Ruby on Rails 有概念的話，上述的 `device` 是一個 Model 物件，也因此我們可以利用 Rails 的 Concern 機制來進行改善。

將 `#load_info` 放到一個 `Devices::HasThirdPartyInfo` 之類的模組，然後提供一個 `#get_firmware_with_third_party` 之類的方法。

> 在 Rails 中 `ActiveSupport::Concern` 屬於一種語法糖，即使是 Ruby 原生的 Module 也足以做到 Mixin（混合）的功能。

也因此我們可以將原本這行

```ruby
  refreshed_firmwares = @devices.map { |device| load_info(device) }
```

變成像這樣

```ruby
  refreshed_firmwares = @devices.map(&:get_firmware_with_third_party)
```

除了可以減少原本物件的資訊數量，也讓每一個物件的職責更加清晰也提高了內聚的程度。

也就是說，最後好好整理完的程式大概會是像這樣的感覺。

```ruby
class FirmwareRefresher
  def initialize
    # ...
  end

  def perform
    refreshed_firmwares = @devices.map(&:get_firmware_with_third_party)
    update_all(refreshed_firmwares)
  rescue Vendor::RequestError => e
    ErrorReporter.capture(e)
  end
end
```

## 後記{#postscript}

文章中還有許多細節沒有討論到，像是如何讓程式可以被測試、在介面的約定（Ex. Code Signature）的設計等等都需要被加入到討論裡面，才算是完整的設計。

很多時候我們會因為習慣而漏掉一些細節，一段時間複習這些知識會是一個很不錯的方式，不過也不要過於仰賴書中或文章的做法，而是去學習在這個過程中「思考」的方式才能理解如何對應不同情境的差異。

我自己在寫程式上的追求主要還是落在清晰、簡單上，也看過不少非常有巧思的程式撰寫方式（最近讀最多的 mruby 原始碼就是其中一個）如果認真的去思考，很多時候我們的程式不一定要做的那麼複雜。

有些時候我看到一些專案的寫法會有一種「原來能這樣做！」的驚嘆，就是因為這些工程師仔細的了解他們所使用的語言、需求，以一種不多也不少的方式去實現，才會讓程式看起來簡潔又容易理解。