做 Python 代碼重構,竟發現這些「潛規則」!
關鍵時刻,第一時間送達!
我們的代碼庫混亂得一塌糊塗。所以有一天,我們決定做一些改進:也就是移動一堆文件。於是,我們花了兩個月的時間完成了此次重構。
【編者按】本文作者Craig Silverstein 寫了一系列文章來記錄可汗學院在 2017 年和 2018 年對 Python 代碼進行的重大重構,本文為其中第一篇。在本文中,作者介紹了代碼在哪些地方出現問題,想要努力修正的內容,以及這項工作為何如此之困難,從而來幫助人們利用本文中介紹的技巧和代碼來避免這些困難。
註:可汗學院(Khan Academy),是由孟加拉裔美國人薩爾曼·可汗創立的一家教育性非營利組織,主旨在於利用網路視頻進行免費授課,現有關於數學、歷史、金融、物理、化學、生物、天文學等科目的內容。
以下為正文:
我們的代碼庫混亂得一塌糊塗。所以有一天,我們決定做一些改進:也就是移動一堆文件。於是,我們花了兩個月的時間完成了此次重構。
我們的代碼庫究竟有什麼問題?可能包括:
有一些文件名為 parent.py,coaches/parent.py,users/parent.py 和 api/internal/parent.py,每個文件都包含了有關可汗學院學生家長的部分邏輯。
有一個名為 login.login.Login.login 的方法。 (這個方法是登錄過程的一部分。)
根目錄有 234 個 python 文件。超過 10%的代碼保存在根目錄。不看也知道,很多文件之間根本毫無關聯。其中有 ip_util.py 等底層工具,也有 dismissed_items.py 等奇怪的特殊工具,還有 activity_summary.py 之類的應用程序邏輯文件,以及 appengine_stats.py 等開發工具。我們還有 93 個 Python 子目錄。 (另外還有一些非 Python 代碼。)雖然根目錄被各種文件塞滿,但是其中 30 個子目錄僅包含不到 5 個文件。
▌這些問題都是如何發生的?
說的冠冕堂皇一點就是日積月累。大家在創建子目錄的時候,以為那個目錄會成為重大項目,但是事實卻並非如此。而大家把新建的文件放在根目錄下的原因是根目錄下有個相似的文件。最終問題就一點點積累起來了。在找不到與手頭的工作相關的已有代碼時,我們一般都會在新文件中寫新的代碼。而當代碼的組織變得毫無道理時(與公司當前的產品或與組織結構不相關時),大家找不到更好的地方保存新文件,所以就隨便放了。沒有人專門負責代碼的結構,代碼永遠也得不到任何修整與重塑。
▌我們進行了哪些改善?
我們在 2017 年底預留了 6 周時間,讓整個公司集中精力解決技術債務,因此我們 3 人決定利用這段時間來重塑代碼庫(Python 部分)。
註:我們只重構了 Python,因為 Javascript 代碼已經組織得很好了。JS 代碼組織得越好,用戶的下載包就越緊湊,且用戶體驗越快。因此,JS 代碼在提高性能的時候已被清理得很整齊,而 Python 代碼還很亂。
我們從子目錄著手。一位熟悉大部分代碼庫的高級工程師設計了初步的目錄結構。其中一些用於 test_prep 或 translation 等產品。其他一些則用於 coaches 或 login 等主要數據結構和工作流程。還有一些用於郵件或 pubsub 等底層的基礎設施。然後他們花了兩天時間將代碼庫中的每個文件都轉入了這些目錄中。有幾十個無法分類的文件,暫時標記為 TODO。
經過這次粗略的修整後,真正的工作才拉開序幕。另一個組重新瀏覽了代碼庫中的每個文件,並根據需要進行重新分類。在這個過程中,有些目錄被刪除了,有些新的目錄加了進來,還有些目錄被分割或合併了。經過更仔細的檢查後,文件最終被轉移到別的目錄下。期間經常遇到文件被拆分的情況,因為它們保存了不相關的代碼。(尤其是一些較大的文件被分到 4 個不同的目錄中!)這項工作沒有捷徑,我們幾乎把代碼庫中所有代碼都看了一遍。
這一步中最有意思的事情是我們如何重新考慮代碼庫的一些部分。例如,以前我們的 emails 目錄保存了可以向用戶發送的所有類型的電子郵件。經過此次分析後,我們意識到如果 emails(現在改名為 email)中只包含郵件發送框架的話,代碼會更整潔,並且每個具體的郵件應該與其最密切的代碼保存在一起(比如 SAT 相關的郵件保存在 sat 中,新用戶的郵件保存在 login,等等)。與之類似,我們將 API 處理程序從 api 目錄中拿出來,放到各自相關的項目目錄中。這兩種組織方式各有利弊,但是之前我們想都沒想就採用了前者。在重構過程中,我們重新考量了這兩種方案。
▌重構工作的難點
代碼審查。雖然 slicker 很好用,但是有些模稜兩可的情況下,每個變化都必須經過仔細審查。例如,我們將 feeds.py 重命名為 content_render/rss/feeds.py。那麼考慮如下代碼:
_MODULE_WHITELIST = ["feeds",…]
user_data ["feeds"] = _get_feeds_for_user()
對於第一行代碼,我們希望把 feeds 改成 content_render.rss.feeds,但不希望變更第二行代碼。自動化工具無法掌控這一點,所以我們只能靠代碼審查(當然還有測試)。我曾經一整天一整天審查代碼。
代碼合併衝突。誰都沒有想到我們會在解決代碼合併衝突上花費如此多的時間。我們預料到了在移動文件的時候,如果有人用到我們的代碼庫,那麼可能出現代碼合併衝突,所以我們小心翼翼地與合作夥伴協調,盡量避免這種情況。但是沒想到所有的合併衝突實際上都發生在我們內部。我們以為可以同時進行多個文件的重命名,但是後來在合併的時候遇到了大量的衝突。這是因為移動文件時會更改其他文件中的 import 語句。假設 somefile.py 同時 import foo 和 import bar,那麼如果一個人重命名 foo.py,而另一個人重命名 bar.py,那麼這兩個人會同時編輯 somefile.py。第二個完成重命名的人必須合併前一個人的變更。不幸的是,import 語句往往集中在一起,而 git 將這兩個更改視為衝突(儘管它們確實不是)。每次這樣的衝突都必須手動解決。解決代碼合併衝突的時間實際上超過了(有時甚至大幅超出)當初創建提交的時間。
目錄結構隱藏的依賴關係。我們有很多的文件包含類似 datafile = os.path.join(__ file__,"..","testdata","whatever")的代碼。重構會導致這些代碼無法正常運行,而修復這些問題的工作非常枯燥。
Pickle。pickle 庫 pickle 類或函數時,它會使用全名:path.to.module.myfunc。如果這個類或函數移動了,那數據就不能 unpickle 了。雖然 pickle 的這個問題不足為奇,但有個特殊的問題是無解的——如果想保存函數名以便後面調用,那麼就必須想辦法指明函數名。事實上,我們只是需要在 AppEngine 的延遲調用庫中這麼做,但是 AppEngine 在我們的代碼庫中用到了很多次。在我們移動文件的時候,這些應用都無法正常工作了。
我們使用了 pickle_util 來解決這個問題。pickle_util 是個 pickle 的 wrapper,在可汗學院代碼的重構中,我們利用它來記錄符號重命名。當 pickle_util 的 unpickle 發現無法導入的類、函數或其他東西時,它會查找符號表以確定符號的新位置,然後再試著從新位置導入。這樣 unpickle 就無需考慮移動文件的問題了。源代碼的鏈接如下,稍加改動就可以用在其他項目上(可以在 pickle 和 cPickle 上使用,但是僅在 python2 上測試過):
http://engineering.khanacademy.org/supporting-files/pickle_util.py
但是我們遇到一個問題:我們不知道需要用 pickle_util 記錄哪些符號,因為我們沒有一個列表指明哪些函數或類可能被 pickle 過。雖然可以用 pickle_util 記錄代碼中的所有符號,但這種做法又笨又慢。所以,我們使用了另一種解決方案:pickle guards。
每次將一個文件移動到新位置時,我們就創建一個 pickle guards 用來「轉發」。pickle guard 文件依然保存在舊位置,但它會從新的位置導入文件。這個轉發文件永遠不會被我們的代碼導入(因為所有的引用都參照新位置),但是 pickle 會使用轉發文件,因為在 unpickle 符號的時候引用的是舊位置。如果轉發文件被導入,我們就會在日誌中寫入一條消息:「pickle 用到了這個文件的符號!」之後,我們可以查看日誌找出需要在 pickle_util 中記錄的東西。日誌中的內容全部處理完後,我們就可以刪除這些 pickle guard 文件,並擁有一個漂亮乾淨的代碼庫。
下面是我們重命名時記錄的 pickle guard 文件的例子:
google_analytics.py to analytics/google_analytics.py:
logging.error("Should not be importing %s, "
"update pickle_util.py", file)
from analytics.google_analytics import _construct_event_payload # NoQA: F401
from analytics.google_analytics import _fix_payload_unicode # NoQA: F401
from analytics.google_analytics import _send_event_to_ga_sync # NoQA: F401
from analytics.google_analytics import google_analytics_user_id # NoQA: F401
from analytics.google_analytics import mark_ga_activation # NoQA: F401
from analytics.google_analytics import send_event_to_ga # NoQA: F401
我們在移動文件時會創建 pickle guard 文件。這個操作的自動化處理代碼鏈接如下:
http://engineering.khanacademy.org/supporting-files/generate_pickle_guards.py
▌自我反省
我認為,很多事情我們都處理得很好。我們在動手之前很明智地總結了具體的工作內容;即便不用深究代碼中的語義,單是移動文件本身的機制就極其錯綜複雜。
我們沒有試圖找一天時間禁止任何人修改核心產品,以避免代碼合併衝突,並利用這段時間做完所有的工作,這一點還是很明智的。我們曾經考慮過,在專門的時間段內做重大修改雖然看上去很不錯,但終歸不是個好主意。在重構過程中,出現了很多意想不到的問題,似乎我們需要一個月的時間才能處理完。好在我們從一開始就盡量圍繞其他人的日程安排進行了規劃,從而避免了與其他開發人員的代碼發生合併衝突,同時又不阻礙別人的工作。
但是,現在回想起來,我認為我們可以投入更多的工具,以便自動解決代碼合併衝突,那我們就不需要不斷修復 import 問題了。這部分工作耗時且容易出錯,應該可以用自動化處理。
還有一點,我認為我們生成的 pickle guard 與移動文件的改動應該分開提交。實際工作中,我們同時進行了這兩項工作,以致 git 無法識別文件移動,而是認為我們編輯了 google_analytics.py 文件(刪掉舊內容,並寫入了 pickle-guard 的內容),並創建了一個看似無關的名為 analytics / google_analytics.py 的新文件。現在 git blame 和 git log 已無法正常工作。(我們可以使用 git blame -C,它可以正常工作,但速度很慢。而 git log 的問題目前還沒有解決辦法。)如果當初我們分開提交,就可以很容易地避免這個問題。
▌兩個月後
清理工作完成後,亂糟糟的代碼總有可能隨時回來。但迄今為止我們還未發現異常情況。代碼組織越清晰,新代碼的去向位置就越明顯,且團隊更加容易將代碼集中在與項目相關的一兩個目錄中。當然,隨著公司新產品的推出與舊產品的下架,代碼結構需要進行調整,但是現在這項工作的難度已經大大降低。而且工作了很長時間的工程師可以更輕鬆地找到自己的代碼。而對於我們這些新員工,只是還沒有感受到這些好處。
原文:http://engineering.khanacademy.org/posts/python-refactor-1.htm
作者:Craig Silverstein
譯者:彎月
責編:張偉


※連設計圖都不會畫,你還想做「系統架構師」?
※如何僅憑 README 就名列 GitHub No.1 並收穫上萬 Star?
TAG:CSDN |