一直以来,程序员热衷于重构旧代码,认为这是必要的。
然而,大多数情况下,他们重构后的代码反而变得更难理解和维护,通常还更慢、更容易出错。
当然,重构本身并不是坏事。保持代码库健康,重构是关键的一环。问题在于,糟糕的重构往往会适得其反,而要避免陷入这种陷阱并不容易。
下面,让我们探讨一下什么样的重构是好的,什么样的是不好的,以及如何避免成为那个让人头疼的开发者。
重构中的优点、缺点、陷阱
抽象有时是有益的,但有时却会带来负面影响。
关键在于了解何时以及如何正确地应用抽象。
来看看一些常见的陷阱,以及如何避免它们。
1. 大幅改变编码风格
一个常见的错误是,开发人员在重构代码时完全改变了编码风格。这种情况通常发生在开发人员来自不同的背景,或者对某种编程范式有强烈的看法时。
举个例子,假设我们有一段代码需要清理。
以下代码处理用户数据。
它接受一个用户数组users,然后遍历这个数组,将年龄大于等于18岁的用户进行格式化,最后返回一个新的用户数组。
Before:
function processUsers(users: User[]){
const result =[];
for(let i =0; i < users.length; i++){
if(users[i].age>=18){
const formattedUser ={
name: users[i].name.toUpperCase(),
age: users[i].age,
isAdult:true
};
result.push(formattedUser);
}
}
return result;
}
糟糕的重构:
import *as R from'ramda';
// 采用了完全不同的风格 + 库
const processUsers = R.pipe(
R.filter(R.propSatisfies(R.gte(R.__,18),'age')),
R.map(R.applySpec({
name: R.pipe(R.prop('name'), R.toUpper),
age: R.prop('age'),
isAdult: R.always(true)
}))
);
虽然这个重构版本可能会吸引函数式编程爱好者,但它引入了一个新的库(Ramda)和完全不同的编码风格。对于不熟悉这种方法的团队来说,维护起来可能是一场噩梦。
好的重构:
// ? 更简洁且更符合惯例
functionprocessUsers(users: User[]):FormattedUser[]{
return users
.filter(user => user.age>=18)
.map(user =>({
name: user.name.toUpperCase(),
age: user.age,
isAdult:true
}));
}
此版本通过使用更惯用的JavaScript方法(如filter和 map)改进了原始代码。它更简洁、更易于阅读,但并没有引入全新的范式或外部依赖关系。
2. 不必要的抽象
我曾经见过一个人,他在不理解底层代码的情况下添加了大量新的抽象。
他们开始对不应该分组的事物进行分组,并且随着时间的推移(故意)分化。他们整合了一些不应该的配置(不同的 API 需要不同的配置)。
Before:
function processUsers(users: User[]){
const result =[];
for(let i =0; i < users.length; i++){
if(users[i].age>=18){
const formattedUser ={
name: users[i].name.toUpperCase(),
age: users[i].age,
isAdult:true
};
result.push(formattedUser);
}
}
return result;
}
糟糕的重构:
// 这里的层次和抽象比实际需要的多得多。
classUserProcessor{
privateusers:User[];
constructor(users: User[]){
this.users= users;
}
publicprocess():FormattedUser[]{
returnthis.filterAdults().formatUsers();
}
privatefilterAdults():UserProcessor{
this.users=this.users.filter(user => user.age>=18);
returnthis;
}
privateformatUsers():FormattedUser[]{
returnthis.users.map(user =>({
name:this.formatName(user.name),
age: user.age,
isAdult:true
}));
}
privateformatName(name:string):string{
return name.toUpperCase();
}
}
const processUsers =(users:User[]):FormattedUser[]=>{
returnnewUserProcessor(users).process();
};
这种重构引入了一个具有多种方法的类,这可能看起来更“面向对象”,但实际上它更复杂,一目了然。
好的重构:
// ? cleaner and more conventional
const isAdult =(user:User):boolean => user.age>=18;
const formatUser =(user:User):FormattedUser =>({
name: user.name.toUpperCase(),
age: user.age,
isAdult:true
});
functionprocessUsers(users: User[]):FormattedUser[]{
return users.filter(isAdult).map(formatUser);
}
此版本将逻辑分解为小的、可重用的函数,而不会引入不必要的复杂性。
3. 添加不一致
开发人员有时候会更新代码库的一部分,结果让它跟其他部分的风格完全不同,试图“优化”一下。
这种做法常常让其他开发人员感到困惑和沮丧,因为他们得在不同的风格间来回切换。
比如说,我们有一个React应用,一直用React Query来获取数据:
// 在整个应用中
import{ useQuery }from'react-query';
functionUserProfile({ userId }){
const{data: user, isLoading }=useQuery(['user', userId], fetchUser);
if(isLoading)return<div>Loading...</div>;
return<div>{user.name}</div>;
}
现在,想象一下,一个开发人员决定只对一个组件使用Redux Toolkit:
// One-off component
import{ useEffect }from'react';
import{ useDispatch, useSelector }from'react-redux';
import{ fetchPosts }from'./postsSlice';
functionPostList(){
const dispatch =useDispatch();
const{ posts, status }=useSelector((state) => state.posts);
useEffect(() =>{
dispatch(fetchPosts());
},[dispatch]);
if(status ==='loading')return<div>Loading...</div>;
return<div>{posts.map(post => <div key={post.id}>{post.title}</div>)}</div>;
}
这种不一致让人感到沮丧,因为它只为一个组件引入了完全不同的状态管理方式。
更好的方法是继续使用React Query:
// 一致的方法
import{ useQuery }from'react-query';
functionPostList(){
const{data: posts, isLoading }=useQuery('posts', fetchPosts);
if(isLoading)return<div>Loading...</div>;
return<div>{posts.map(post => <div key={post.id}>{post.title}</div>)}</div>;
}
这个版本保持了一致性,整个应用程序都使用React Query来获取数据。这样做更简单,不需要其他开发人员去学习一个新组件的不同模式。
保持代码库的一致性非常重要。如果需要引入新的模式,最好是重构整个代码库以使用这个新模式,而不是仅仅为了一个组件而引入不一致的方式。
4. 重构前不理解代码
在学习代码的过程中进行重构,这通常是一个不明智的做法。
有建议,在使用一段给定的代码时,最好保持不变6到9个月。
否则,频繁的重构可能导致错误的引入、性能问题等。
Before:
// 这里硬编码的内容有点过多。
functionfetchUserData(userId: string){
const cachedData =localStorage.getItem(`user_${userId}`);
if(cachedData){
returnJSON.parse(cachedData);
}
return api.fetchUser(userId).then(userData =>{
localStorage.setItem(`user_${userId}`,JSON.stringify(userData));
return userData;
});
}
糟糕的重构:
// 缓存去哪了?
function fetchUserData(userId: string) {
return api.fetchUser(userId);
}
重构者可能认为他们在简化代码,但他们实际上已经删除了一个重要的缓存机制,该机制是为了减少 API 调用和提高性能而存在的。
好的重构:
// ? 保持现有功能的同时,使代码更简洁
async function fetchUserData(userId: string){
const cachedData =await cacheManager.get(`user_${userId}`);
if(cachedData){
return cachedData;
}
const userData =await api.fetchUser(userId);
await cacheManager.set(`user_${userId}`, userData,{expiresIn:'1h'});
return userData;
}
此重构可维护缓存行为,同时可能通过使用具有过期功能的更复杂的缓存管理器来改进缓存行为。
5. 了解业务背景
比如目标是完善一套糟糕的遗留代码,并带领项目将电商公司迁移到新的技术上——Angular.js。
结果发现,公司非常依赖SEO,而却做了一个又慢又臃肿的单页应用。
于是只能交付了一个更慢、更容易出错、难以维护的网站。
为什么?因为负责这个项目的人,对这个网站的实际的核心需求一无所知。
糟糕的重构:
// a single page app for an SEO-focused site is a bad idea
function App() {
return (
<Router>
<Switch>
<Route path="/product/:id" component={ProductDetails} />
</Switch>
</Router>
);
}
这种方法可能看起来很现代和干净,但它完全是客户端呈现的。对于一个严重依赖SEO的电子商务网站来说,这可能是灾难性的。
好的重构:
// ? server render an SEO-focused site
exportconstgetStaticProps:GetStaticProps=async()=>{
const products =awaitgetProducts();
return{props:{ products }};
};
exportdefaultfunctionProductList({ products }){
return(
<div>
...
</div>
);
}
这种基于Next.js的方法提供了开箱即用的服务器端渲染,这对于SEO至关重要。它还提供了更好的用户体验,初始页面加载速度更快,并为连接速度较慢的用户提高了性能。Remix同样适用于此目的,为服务器端渲染和SEO优化提供类似的好处。
6. 过度整合代码
再看一个例子,比如有一堆Firebase函数,其中一些函数的设置与其他函数不同——例如超时和内存分配。
Before:
// we had this same code 40+ times in the codebase, we could perhaps consolidate
exportconst quickFunction = functions
.runWith({timeoutSeconds:60,memory:'256MB'})
.https.onRequest(...);
exportconst longRunningFunction = functions
.runWith({timeoutSeconds:540,memory:'1GB'})
.https.onRequest(...);
程序员决定将所有这些函数包装在一个 createApi 函数中。
糟糕的重构:
// blindly consolidating settings that should not be
constcreateApi=(handler: RequestHandler)=>{
return functions
.runWith({timeoutSeconds:300,memory:'512MB'})
.https.onRequest((req, res) =>handler(req, res));
};
exportconst quickFunction =createApi(handleQuickRequest);
exportconst longRunningFunction =createApi(handleLongRunningRequest);
此重构将所有API设置为具有相同的设置,并且无法覆盖每个API。这是一个问题,因为有时我们需要为不同的功能设置不同的设置。
更好的方法是允许Firebase选项按API传递。
好的重构:
// ? setting good defaults, but letting anyone override
constcreateApi=(handler: RequestHandler, options: ApiOptions = {})=>{
return functions
.runWith({timeoutSeconds:300,memory:'512MB',...options })
.https.onRequest((req, res) =>handler(req, res));
};
exportconst quickFunction =createApi(handleQuickRequest,{timeoutSeconds:60,memory:'256MB'});
exportconst longRunningFunction =createApi(handleLongRunningRequest,{timeoutSeconds:540,memory:'1GB' });
在进行代码抽象或重构时,确保保留原始功能和灵活性。不要为了“清晰”而牺牲灵活性。改进之前要彻底理解代码,以避免引入新问题。
如何正确地进行重构
重构是必要的,但要确保方法正确。
虽然我们的代码并不完美,确实需要优化,但重构必须与现有代码库保持一致,并建立在对代码的深刻理解和对抽象的审慎选择之上。
以下是实现成功重构的一些指导:
1、循序渐进:进行小而可控的更改,而非一次性的大规模重写。
2、深入理解代码:在进行重大重构或引入新抽象之前,确保彻底理解现有代码的运作方式。
3、保持风格一致:遵循现有代码风格,一致性是可维护性的基础。
4、慎用新抽象:除非复杂性有充分理由,否则保持代码简单明了。
5、避免轻率引入新库:尤其是风格迥异的库,在没有团队一致支持的情况下不要随意引入。
6、测试驱动重构:在重构前编写测试,并在重构过程中及时更新测试,确保原有功能的完整性。
7、团队协作:确保同事们也遵循这些原则,共同为代码库的长期健康负责。
用于更好地重构的工具和技术
为了确保重构是有益的而非有害的,可以考虑以下技术和工具:
Linting tools
使用linting工具来强制执行一致的代码风格并捕捉潜在问题。Prettier可以自动格式化代码以保持风格统一,而ESLint则可进行更深入的代码一致性检查,并允许通过插件进行自定义,以适应团队的特定需求。
Code reviews
在合并重构代码之前,实施严格的代码审查,确保从团队成员处获得反馈。这有助于及早发现潜在问题,并确保重构后的代码符合团队的标准和预期。
Testing
通过编写和运行测试,确保重构不会破坏现有功能。Vitest是一个快速、可靠且易于使用的测试框架,默认情况下无需配置。对于UI测试,可以考虑使用Storybook。React Testing Library是一组出色的工具,用于测试React组件,此外还有适用于Angular等其他框架的变体。
AI tools
利用AI辅助重构,尤其是在与现有代码风格和约定保持一致的情况下。
Visual Copilot是一个在编写前端代码时特别有用的AI工具。
它可以帮助将设计转化为代码,同时确保与现有编码风格保持一致,并正确使用设计系统的组件和样式规范。
结论
重构是软件开发中不可或缺的一环,但需要谨慎对待,尤其要尊重现有代码库的结构和团队的协作方式。
重构的核心目标是优化代码的内部结构,而不影响其外部行为。
需要时刻牢记,最成功的重构通常是对用户不可见的,但却能显著简化开发过程。
它们提高了代码的可读性、可维护性和效率,同时避免了对系统的重大干扰。
所以,当你有意对某段代码进行“大刀阔斧”的改动时,不妨先退一步。
深入理解代码,评估更改的影响,并采取逐步改进的方法。
这样的方式不仅有利于代码库的长期健康,你和你的同事在未来也会感激这一深思熟虑的策略。
参考:https://www.builder.io/blog/good-vs-bad-refactoring