如何重构一些可怕的代码

gtsjx 2020-05-19

即将开播:5月20日,基于kubernetes打造企业级私有云实践

最近,我不得不重构一些处于粗糙状态的代码:

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

如何重构一些可怕的代码

相关推荐