一篇聊透好重構與壞重構
這些年來,我雇傭過很多開發人員。他們當中有很多人都堅信我們的代碼需要大量重構。但問題是:幾乎在每一個案例中,其他開發人員都發現他們新重構的代碼更難理解和維護。此外,代碼的運行速度通常也更慢,漏洞也更多。
別誤會我的意思。重構本質上并不是壞事。它是保持代碼庫健康的關鍵部分。問題是,糟糕的重構就是糟糕。而且,在試圖讓事情變得更好的同時,我們很容易掉入讓事情變得更糟的陷阱。
因此,讓我們來看看什么是好的重構,什么是壞的重構,以及如何避免成為大家都害怕在代碼庫附近看到的那個開發人員。
重構的好與壞
抽象可以是好的。抽象可以是壞的。關鍵是要知道何時以及如何應用抽象。讓我們來看看一些常見的陷阱以及如何避免它們。
1. 大幅改變編碼風格
我見過的最常見的錯誤之一,就是開發人員在重構過程中完全改變編碼風格。這種情況通常發生在來自不同背景的人或對特定編程范式有強烈意見的人身上。
讓我們來看一個例子。假設我們有一段代碼需要清理:
重構之前:
// ?? this code could be cleaner
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';
// ?? adopted a completely different style + library
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)和一種完全不同的編碼風格。對于不熟悉這種方法的團隊來說,維護起來可能是一場噩夢。
好的重構:
// ? cleaner and more conventional
function processUsers(users: User[]): FormattedUser[] {
return users
.filter(user => user.age >= 18)
.map(user => ({
name: user.name.toUpperCase(),
age: user.age,
isAdult: true
}));
}
該版本改進了原始代碼,使用了 filter 和 map 等更多慣用的 JavaScript 方法,更加簡潔易讀,但并未引入全新的范式或外部依賴關系。
2.不必要的抽象
我曾經雇傭過一個人,他在不了解底層代碼的情況下添加了大量新的抽象概念。他們開始將不應該分組的東西分組,并且隨著時間的推移(故意)產生分歧。他們合并了一些不該合并的配置(不同的 API 需要不同的配置)。
重構之前:
// ?? this code could be cleaner
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;
}
壞的重構:
// ?? there are way more layers and abstractions here than necessary
class UserProcessor {
private users: User[];
constructor(users: User[]) {
this.users = users;
}
public process(): FormattedUser[] {
return this.filterAdults().formatUsers();
}
private filterAdults(): UserProcessor {
this.users = this.users.filter(user => user.age >= 18);
return this;
}
private formatUsers(): FormattedUser[] {
return this.users.map(user => ({
name: this.formatName(user.name),
age: user.age,
isAdult: true
}));
}
private formatName(name: string): string {
return name.toUpperCase();
}
}
const processUsers = (users: User[]): FormattedUser[] => {
return new UserProcessor(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
});
function processUsers(users: User[]): FormattedUser[] {
return users.filter(isAdult).map(formatUser);
}
該版本將邏輯分解為可重復使用的小函數,而不會引入不必要的復雜性。
3.增加不一致性
我曾見過這樣的情況:開發人員更新代碼庫的一部分,使其工作方式與其他部分完全不同,試圖使其 "更好"。這往往會給其他開發人員帶來困惑和挫敗感,因為他們不得不在不同風格之間進行上下文切換。
假設我們有一個 React 應用程序,在該應用程序中,我們始終使用 React Query 來獲取數據:
// Throughout the app
import { useQuery } from 'react-query';
function UserProfile({ userId }) {
const { data: user, isLoading } = useQuery(['user', userId], fetchUser);
if (isLoading) return <div>Loading...</div>;
return <div>{user.name}</div>;
}
現在,想象一下開發人員決定只在一個組件中使用 Redux 工具包:
// One-off component
import { useEffect } from 'react';
import { useDispatch, useSelector } from 'react-redux';
import { fetchPosts } from './postsSlice';
function PostList() {
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:
// Consistent approach
import { useQuery } from 'react-query';
function PostList() {
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 個月的時間來處理一段特定的代碼。否則,你很可能會產生錯誤、影響性能等。
重構之前:
// ?? a bit too much hard coded stuff here
function fetchUserData(userId: string) {
const cachedData = localStorage.getItem(`user_${userId}`);
if (cachedData) {
return JSON.parse(cachedData);
}
return api.fetchUser(userId).then(userData => {
localStorage.setItem(`user_${userId}`, JSON.stringify(userData));
return userData;
});
}
壞的重構:
// ?? where did the caching go?
function fetchUserData(userId: string) {
return api.fetchUser(userId);
}
重構者可能會認為他們在簡化代碼,但實際上他們已經刪除了一個重要的緩存機制,而該機制是為了減少 API 調用和提高性能而設置的。
好的重構:
// ? cleaner code preserving the existing behavior
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
事實證明,這項業務在很大程度上依賴于搜索引擎優化,而我們構建了一個緩慢而臃腫的單頁面應用程序。
兩年來,我們除了提供一個速度更慢、漏洞更多、可維護性更差的網站復制品外,什么也沒提供。為什么會這樣?領導這個項目的人(我--我是這個場景中的混蛋)以前從未在這個網站上工作過。我當時又年輕又笨。
讓我們來看一個現代錯誤的例子:
壞的重構:
// ?? 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>
);
}
這種方法看似現代簡潔,但完全是客戶端渲染。對于嚴重依賴搜索引擎優化的電子商務網站來說,這可能是災難性的。
好的重構:
// ? server render an SEO-focused site
export const getStaticProps: GetStaticProps = async () => {
const products = await getProducts();
return { props: { products } };
};
export default function ProductList({ products }) {
return (
<div>
...
</div>
);
}
這種基于 Next.js 的方法提供開箱即用的服務器端渲染,這對搜索引擎優化至關重要。它還能提供更好的用戶體驗,加快初始頁面加載速度,并為連接速度較慢的用戶提高性能。Remix 也同樣適用于這一目的,在服務器端渲染和搜索引擎優化方面具有類似的優勢。
6.過度合并代碼
我曾經雇過一個人,他第一天在我們的后臺工作,就立即開始重構代碼。我們有很多 Firebase 函數,有些函數的設置與其他函數不同,比如超時和內存分配。
這是我們最初的設置。
重構之前:
// ?? we had this same code 40+ times in the codebase, we could perhaps consolidate
export const quickFunction = functions
.runWith({ timeoutSeconds: 60, memory: '256MB' })
.https.onRequest(...);
export const longRunningFunction = functions
.runWith({ timeoutSeconds: 540, memory: '1GB' })
.https.onRequest(...);
這個人決定將所有這些函數封裝在一個 createApi 函數中。
壞的重構:
// ?? blindly consolidating settings that should not be
const createApi = (handler: RequestHandler) => {
return functions
.runWith({ timeoutSeconds: 300, memory: '512MB' })
.https.onRequest((req, res) => handler(req, res));
};
export const quickFunction = createApi(handleQuickRequest);
export const longRunningFunction = createApi(handleLongRunningRequest);
這次重構將所有 API 設置為相同的設置,而無法覆蓋每個 API。這是個問題,因為有時我們需要對不同的函數進行不同的設置。
更好的方法是允許 Firebase 選項通過每個應用程序接口傳遞。
好的重構:
// ? setting good defaults, but letting anyone override
const createApi = (handler: RequestHandler, options: ApiOptions = {}) => {
return functions
.runWith({ timeoutSeconds: 300, memory: '512MB', ...options })
.https.onRequest((req, res) => handler(req, res));
};
export const quickFunction = createApi(handleQuickRequest, { timeoutSeconds: 60, memory: '256MB' });
export const longRunningFunction = createApi(handleLongRunningRequest, { timeoutSeconds: 540, memory: '1GB' });
這樣,我們既能保持抽象的優勢,又能保留我們所需的靈活性。在合并或抽象時,請始終考慮你所服務的用例。不要為了 "更簡潔" 的代碼而犧牲靈活性。確保你的抽象實現了原始實現所提供的全部功能。
說真的,在開始 "改進" 代碼之前,請先了解代碼。我們在下一次部署一些應用程序接口時就遇到了問題,如果不進行盲目的重構,這些問題是可以避免的。
如何正確重構
值得注意的是,你確實需要重構代碼。但要正確對待。我們的代碼并不完美,我們的代碼需要清理,但要與代碼庫保持一致,熟悉代碼,對抽象要精挑細選。
下面是一些成功重構的技巧:
- Be incremental: Make small, manageable changes rather than sweeping rewrites.循序漸進:進行小規模、可控的修改,而不是大刀闊斧的改寫。
- Deeply understand code before doing significant refactors or new abstractions.在進行重大重構或新抽象之前,深入理解代碼。
- Match the existing code style: Consistency is key for maintainability.與現有代碼風格相匹配:一致性是可維護性的關鍵。
- Avoid too many new abstractions: Keep it simple unless complexity is truly warranted.避免過多的新抽象概念:保持簡單,除非確實需要復雜。
- Avoid adding new libraries, especially of a very different programming style, without buy-in from the team.避免在未獲得團隊認可的情況下添加新的庫,尤其是編程風格迥異的庫。
- Write tests before refactoring and update them as you go. This ensures you're maintaining the original functionality.在重構前編寫測試,并在重構過程中更新測試。這能確保你保持原有的功能。
- Hold your coworkers accountable to these principles.讓你的同事對這些原則負責。
圖片
更好地重構的工具和技術
為了確保你的重構是有益而非有害的,請考慮使用以下技術和工具:
規范工具
使用規范工具來執行一致的代碼風格并捕捉潛在的問題。Prettier 可以幫助自動格式化為一致的風格,而 Eslint 則可以幫助進行更細致的一致性檢查,你可以用自己的插件輕松定制。
代碼審查
在合并重構代碼之前,實施全面的代碼審查,以獲得同行的反饋意見。這有助于及早發現潛在問題,并確保重構代碼符合團隊標準和期望。
測試
編寫并運行測試,確保重構代碼不會破壞現有功能。Vitest[1] 是一款快速、可靠、易用的測試運行程序,默認情況下無需任何配置。對于可視化測試,可以考慮使用 Storybook[2]。React Testing Library[3] 是一套用于測試 React 組件的實用工具(還有 Angular 和更多變體)。
人工智能工具
讓人工智能來幫助你進行重構,至少是那些能夠與你現有的編碼風格和習慣相匹配的重構。
結論
重構是軟件開發的必要組成部分,但在進行重構時需要深思熟慮,并尊重現有的代碼庫和團隊動態。重構的目的是在不改變代碼外部行為的情況下改進代碼的內部結構。
請記住,最好的重構往往是最終用戶看不到的,但卻能大大方便開發人員的工作。它們能提高可讀性、可維護性和效率,而不會破壞整個統。
下一次,當你有為一段代碼制定 "大計劃" 的沖動時,請退后一步。徹底了解它,考慮更改帶來的影響,然后逐步改進,你的團隊一定會感謝你的。
本文譯自:https://www.builder.io/blog/good-vs-bad-refactoring
Reference
[1] Vitest: https://vitest.dev/
[2] Storybook: https://storybook.js.org/
[3] React Testing Library: https://github.com/testing-library/react-testing-library