如何重構一些可怕的代碼
最近,我不得不重構一些處于粗糙狀態的代碼:
- function endpoint(service, version = '') {
- let protocol
- let domain
- if (service === 'webclient') {
- protocol = __CLIENT_PROTOCOL__
- domain = `${__ENV__}-${service}.${__DOMAIN__}`
- if (__SERVER__) {
- protocol = 'http'
- } else if (__ENV__ === 'production') {
- domain = `www.${__DOMAIN__}`
- }
- } else {
- if (__ENV__ !== 'local') {
- if (__SERVER__) {
- protocol = 'http'
- domain = `${__ENV__}-${service}`
- domain += `.${__DOMAIN__}`
- } else {
- protocol = __CLIENT_PROTOCOL__
- domain = `${__ENV__}-api.${__DOMAIN__}`
- if (service !== 'core') {
- domain += `/${service}`
- }
- if (version) {
- domain += `/${version}`
- }
- }
- } else {
- protocol = 'http'
- if (service === 'core') {
- if (__CLIENT__) {
- domain = `api.${__DOMAIN__}`
- } else {
- domain = `api.${__DOMAIN__}:80`
- }
- } else {
- if (__CLIENT__) {
- domain = `api.${__DOMAIN__}/${service}/${version}`
- } else {
- domain = `api.${__DOMAIN__}:80/${service}/${version}`
- }
- }
- }
- }
- const url = `${protocol}://${domain}`
- return url
- }
- export default endpoint
上面的邏輯確定端點的URL,它取決于多種因素:您使用的服務,在服務器或客戶端上呈現的服務,在生產環境或測試環境中使用的服務等。
一段代碼變得如此混亂的原因之一是,當我們忘記重復比錯誤的抽象要便宜得多時。
好消息是,您可以應用幾種簡單的技術來簡化嵌套的if-else語句。 壞消息是,這段代碼對應用程序至關重要,因為所有請求都在調用它。 哦,它也沒有經過測試!
所以,這就是我重構的方式…
1. 給該代碼100%的測試覆蓋率
我沒有執行重構代碼的任務,所以打電話了。 但是我不知道要花多少時間(我本該用來交付其他東西的時間)。
重構本身并不總是合理的。 但是,很難想象有人會抱怨增加測試范圍,尤其是在涵蓋如此關鍵的功能的情況下。 因此,我決定從此開始,不僅是為了讓我對重構充滿信心,而且因為當我完成測試時,我將更好地了解重構的難度,并且我可以進行/不進行。 去打電話。
如今,我們大多數人都在Gousto中實踐TDD,但是代碼庫的這一特定部分是幾年前的,它的重要性和狀態阻礙了我和其他人在過去對其進行重構。
TDD的主要好處是無需擔心,也無需花費成本即可進行重構-羅伯特·馬丁(Robert C. Martin,鮑勃叔叔)
我還想確保覆蓋率是100%,所以我使用了Jest標志–coverage,它提供了以下輸出:
- -------------------|----------|----------|----------|----------|-------------------|
- File | % Stmts | % Branch | % Funcs | % Lines | Uncovered Line #s |
- -------------------|----------|----------|----------|----------|-------------------|
- ... | ... | ... | ... | ... | ...|
- endpoint.js | 100 | 100 | 100 | 100 | |
- ... | ... | ... | ... | ... | ...|
- -------------------|----------|----------|----------|----------|-------------------|
- Test Suites: 1 passed, 1 total
- Tests: 12 passed, 12 total
2. 提取功能
現在我們有了測試,有了更多的信心,我們可以開始拆分代碼。 讓我們從頭開始。 我們看到,根據服務,環境,客戶端/服務器端……為協議分配了不同的值,然后將其添加到函數末尾的其余url中。
- const url = `${protocol}://${domain}`
因此,我們可以將確定協議的代碼抽象為自己的函數,只需調用一次即可:
- import endpoint from 'config/endpoint'
- describe('endpoint.js', () => {
- global.__DOMAIN__ = 'gousto.local'
- let service
- describe('when the service is "webclient"', () => {
- beforeEach(() => {
- service = 'webclient'
- })
- describe('and being in the server side', () => {
- beforeEach(() => {
- global.__SERVER__ = true
- global.__ENV__ = 'whateverenv'
- })
- test('an http address with the corresponding ENV, SERVICE and DOMAIN is returned', () => {
- const url = endpoint(service)
- expect(url).toBe(`http://${__ENV__}-${service}.${__DOMAIN__}`)
- })
- })
- describe('and not being in the server side', () => {
- ...
- describe('and the environment is production', () => {
- ...
- test('an https address with "www" and without the service, but with the DOMAIN is returned', () => {...})
- })
- describe('and the environment is not production', () => {
- ...
- test('an https address with the corresponding ENV, SERVICE and DOMAIN is returned', () => {...})
- })
- })
- })
- describe('when the service is not "webclient"', () => {
- ...
- describe('and the env is not "local"', () => {
- ...
- describe('and being in the server side', () => {
- ...
- test('an http address with the corresponding ENV, SERVICE and DOMAIN is returned', () => {...})
- })
- describe('and not being in the server side', () => {
- ...
- describe('and the service is core', () => {
- ...
- test('an https API address with the corresponding ENV and DOMAIN is returned', () => {...})
- describe('and a version was passed', () => {
- test('an https API address with the corresponding ENV, DOMAIN, SERVICE and VERSION is returned', () => {...})
- })
- })
- describe('and a version was passed', () => {
- test('an https API address with the corresponding ENV, DOMAIN, SERVICE and VERSION is returned', () => {...})
- })
- describe('and a version was not passed', () => {
- test('an https API address with the corresponding ENV, DOMAIN and SERVICE is returned', () => {...})
- })
- })
- })
- describe('and the env is "local"', () => {
- ...
- describe('and the service is core', () => {
- ...
- describe('and being in the client side', () => {
- ...
- test('an http API address with the corresponding DOMAIN is returned', () => {...})
- })
- describe('and not being in the client side', () => {
- ...
- test('an http API address with the corresponding DOMAIN and port 80 is returned', () => {...})
- })
- })
- describe('and the service is not core', () => {
- ...
- describe('and being in the client side', () => {
- ...
- test('an http API address with the corresponding DOMAIN, SERVICE and VERSION is returned', () => {...})
- })
- describe('and not being in the client side', () => {
- ...
- test('an http API address with the corresponding DOMAIN, port 80, SERVICE and VERSION is returned', () => {...})
- })
- })
- })
- })
- })
我們可以對URL的其余部分執行與getProtocol()相同的操作。 提取的功能越多,if-else的地獄就越容易簡化,提取剩余的內容就越容易。 因此,我的建議是從最小的復雜度入手,因為這將使其余的操作更為簡單。
提取這些函數并不復雜,但這是因為我將if-else的噩夢翻譯成了它們。 因此,我們沒有大的混亂,但有一些小混亂。 不能接受 讓我們來照顧它。
3. 簡化
除了關注點分離之外,將url的不同部分提取為不同功能的優點是可以進一步簡化條件。 之前,URL的某些部分限制了簡化,因為它們需要滿足多個條件。 現在,它們是分開的,所以我們不必為此擔心。
您可以只是出于對它的關注而進行簡化……或者您可以使用真值表來提供幫助。
對于getProtocol(),一種真值表如下所示:
但可以簡化一下("-"表示該值無關緊要):
我們只有兩個可能的值:http和https,因此我們可以采用較少行(https)的值,檢查條件,然后返回另一個(http)。
- const getProtocol = (service, isServerSide, environment) => {
- if (service === 'webclient' && !isServerSide) {
- return 'https'
- }
- if (service !== 'webclient' && !isServerSide && environment !== 'local') {
- return 'https'
- }
- return 'http'
- }
這已經比上一節中的要好。 但是我們仍然可以做更多! 檢查前兩個條件,您可能會發現只有在我們不在服務器端時才獲得https。 因此,我們可以為http編寫第一個條件,并在函數的其余部分中擺脫isServerSide:
- const getProtocol = (service, isServerSide, environment) => {
- if (isServerSide) {
- return 'http'
- }
- if (service === 'webclient') {
- return 'https'
- }
- if (service !== 'webclient' && environment !== 'local') {
- return 'https'
- }
- return 'http'
- }
不過,我們可以做得更好。 現在我們可以合并第二個和第三個條件,因為它們較小:
- const getProtocol = (service, isServerSide, environment) => {
- ...
- if (service === 'webclient' ||
- (service !== 'webclient' && environment !== 'local')) {
- return 'https'
- }
- ...
- }
但是,請保持……這種狀況有點愚蠢,不是嗎? 如果服務是webclient,則條件為true。 否則……轉到OR的第二部分,為什么我們需要檢查服務是否不是webclient? 如果我們在OR的右側,那么我們確定它不是webclient。 最終結果看起來很整潔,特別是與第2部分中的原始結果相比。提取功能:
- const getProtocol = (service, isServerSide, environment) => {
- if (isServerSide) {
- return 'http'
- }
- if (service === 'webclient' || environment !== 'local') {
- return 'https'
- }
- return 'http'
- }
結果
多虧了功能的提取,我們最終有了一個endpoint()函數,這是我寫的榮幸。
- ...
- function endpoint(service, version = '') {
- const protocol = getProtocol(service, __SERVER__, __ENV__)
- const subdomain = getSubdomain(service, __SERVER__, __ENV__)
- const path = getPath(service, __SERVER__, __ENV__, version)
- const port = getPort(service, __ENV__, __CLIENT__)
- return `${protocol}://${subdomain}.${__DOMAIN__}${port}${path}`
- }
- export default endpoint
上面代碼的get函數很小,足以被理解而不會動您的大腦。 并非所有人都像我想要的那樣簡單,但是它們比原始代碼要好得多。 您可以在這里看到文件的外觀。
最后,您應該始終嘗試遵循以下規則:讓代碼比發現的要好。 盡管贊美❤️不要這么做
> GitHub comment in the PR
> A team mate making my day
現在輪到您了,您是否有任何想法使它變得更簡單(無需更改在其余應用程序中的使用方式)? 您對重構的處理方式會有所不同嗎?