25種代碼壞味道總結+優化示例
前言
什么樣的代碼是好代碼呢?好的代碼應該命名規范、可讀性強、擴展性強、健壯性......而不好的代碼又有哪些典型特征呢?這25種代碼壞味道大家要注意啦
1. Duplicated Code (重復代碼)
重復代碼就是不同地點,有著相同的程序結構。一般是因為需求迭代比較快,開發小伙伴擔心影響已有功能,就復制粘貼造成的。重復代碼很難維護的,如果你要修改其中一段的代碼邏輯,就需要修改多次,很可能出現遺漏的情況。
如何優化重復代碼呢?分三種情況討論:
同一個類的兩個函數含有相同的表達式
- class A {
- public void method1() {
- doSomething1
- doSomething2
- doSomething3
- }
- public void method2() {
- doSomething1
- doSomething2
- doSomething4
- }
- }
優化手段:可以使用Extract Method(提取公共函數) 抽出重復的代碼邏輯,組成一個公用的方法。
- class A {
- public void method1() {
- commonMethod();
- doSomething3
- }
- public void method2() {
- commonMethod();
- doSomething4
- }
- public void commonMethod(){
- doSomething1
- doSomething2
- }
- }
兩個互為兄弟的子類內含相同的表達式
- class A extend C {
- public void method1() {
- doSomething1
- doSomething2
- doSomething3
- }
- }
- class B extend C {
- public void method1() {
- doSomething1
- doSomething2
- doSomething4
- }
- }
優化手段:對兩個類都使用Extract Method(提取公共函數),然后把抽取出來的函數放到父類中。
- class C {
- public void commonMethod(){
- doSomething1
- doSomething2
- }
- }
- class A extend C {
- public void method1() {
- commonMethod();
- doSomething3
- }
- }
- class B extend C {
- public void method1() {
- commonMethod();
- doSomething4
- }
- }
兩個毫不相關的類出現重復代碼
如果是兩個毫不相關的類出現重復代碼,可以使用Extract Class將重復代碼提煉到一個類中。這個新類可以是一個普通類,也可以是一個工具類,看具體業務怎么劃分吧。
2 .Long Method (長函數)
長函數是指一個函數方法幾百行甚至上千行,可讀性大大降低,不便于理解。反例如下:
- public class Test {
- private String name;
- private Vector<Order> orders = new Vector<Order>();
- public void printOwing() {
- //print banner
- System.out.println("****************");
- System.out.println("*****customer Owes *****");
- System.out.println("****************");
- //calculate totalAmount
- Enumeration env = orders.elements();
- double totalAmount = 0.0;
- while (env.hasMoreElements()) {
- Order order = (Order) env.nextElement();
- totalAmount += order.getAmout();
- }
- //print details
- System.out.println("name:" + name);
- System.out.println("amount:" + totalAmount);
- ......
- }
- }
可以使用Extract Method,抽取功能單一的代碼段,組成命名清晰的小函數,去解決長函數問題,正例如下:
- public class Test {
- private String name;
- private Vector<Order> orders = new Vector<Order>();
- public void printOwing() {
- //print banner
- printBanner();
- //calculate totalAmount
- double totalAmount = getTotalAmount();
- //print details
- printDetail(totalAmount);
- }
- void printBanner(){
- System.out.println("****************");
- System.out.println("*****customer Owes *****");
- System.out.println("****************");
- }
- double getTotalAmount(){
- Enumeration env = orders.elements();
- double totalAmount = 0.0;
- while (env.hasMoreElements()) {
- Order order = (Order) env.nextElement();
- totalAmount += order.getAmout();
- }
- return totalAmount;
- }
- void printDetail(double totalAmount){
- System.out.println("name:" + name);
- System.out.println("amount:" + totalAmount);
- }
- }
3. Large Class (過大的類)
一個類做太多事情,維護了太多功能,可讀性變差,性能也會下降。舉個例子,訂單相關的功能你放到一個類A里面,商品庫存相關的也放在類A里面,積分相關的還放在類A里面...反例如下:
- Class A{
- public void printOrder(){
- System.out.println("訂單");
- }
- public void printGoods(){
- System.out.println("商品");
- }
- public void printPoints(){
- System.out.println("積分");
- }
- }
試想一下,亂七八糟的代碼塊都往一個類里面塞,還談啥可讀性。應該按單一職責,使用Extract Class把代碼劃分開,正例如下:
- Class Order{
- public void printOrder(){
- System.out.println("訂單");
- }
- }
- Class Goods{
- public void printGoods(){
- System.out.println("商品");
- }
- }
- Class Points{
- public void printPoints(){
- System.out.println("積分");
- }
- }
- }
4. Long Parameter List (過長參數列)
方法參數數量過多的話,可讀性很差。如果有多個重載方法,參數很多的話,有時候你都不知道調哪個呢。并且,如果參數很多,做新老接口兼容處理也比較麻煩。
- public void getUserInfo(String name,String age,String sex,String mobile){
- // do something ...
- }
如何解決過長參數列問題呢?將參數封裝成結構或者類,比如我們將參數封裝成一個DTO類,如下:
- public void getUserInfo(UserInfoParamDTO userInfoParamDTO){
- // do something ...
- }
- class UserInfoParamDTO{
- private String name;
- private String age;
- private String sex;
- private String mobile;
- }
5. Divergent Change (發散式變化)
對程序進行維護時, 如果添加修改組件, 要同時修改一個類中的多個方法, 那么這就是 Divergent Change。舉個汽車的例子,某個汽車廠商生產三種品牌的汽車:BMW、Benz和LaoSiLaiSi,每種品牌又可以選擇燃油、純電和混合動力。反例如下:
- /**
- * 公眾號:撿田螺的小男孩
- */
- public class Car {
- private String name;
- void start(Engine engine) {
- if ("HybridEngine".equals(engine.getName())) {
- System.out.println("Start Hybrid Engine...");
- } else if ("GasolineEngine".equals(engine.getName())) {
- System.out.println("Start Gasoline Engine...");
- } else if ("ElectricEngine".equals(engine.getName())) {
- System.out.println("Start Electric Engine");
- }
- }
- void drive(Engine engine,Car car) {
- this.start(engine);
- System.out.println("Drive " + getBrand(car) + " car...");
- }
- String getBrand(Car car) {
- if ("Baoma".equals(car.getName())) {
- return "BMW";
- } else if ("BenChi".equals(car.getName())) {
- return "Benz";
- } else if ("LaoSiLaiSi".equals(car.getName())) {
- return "LaoSiLaiSi";
- }
- return null;
- }
- }
如果新增一種品牌新能源電車,然后它的啟動引擎是核動力呢,那么就需要修改Car類的start和getBrand方法啦,這就是代碼壞味道:Divergent Change (發散式變化)。
如何優化呢?一句話總結:拆分類,將總是一起變化的東西放到一塊。
★ 運用提煉類(Extract Class) 拆分類的行為。
如果不同的類有相同的行為,提煉超類(Extract Superclass) 和 提煉子類(Extract Subclass)。 ”
正例如下:
因為Engine是獨立變化的,所以提取一個Engine接口,如果新加一個啟動引擎,多一個實現類即可。如下:
- //IEngine
- public interface IEngine {
- void start();
- }
- public class HybridEngineImpl implements IEngine {
- @Override
- public void start() {
- System.out.println("Start Hybrid Engine...");
- }
- }
因為drive方法依賴于Car,IEngine,getBand方法;getBand方法是變化的,也跟Car是有關聯的,所以可以搞個抽象Car的類,每個品牌汽車繼承于它即可,如下
- public abstract class AbstractCar {
- protected IEngine engine;
- public AbstractCar(IEngine engine) {
- this.engine = engine;
- }
- public abstract void drive();
- }
- //奔馳汽車
- public class BenzCar extends AbstractCar {
- public BenzCar(IEngine engine) {
- super(engine);
- }
- @Override
- public void drive() {
- this.engine.start();
- System.out.println("Drive " + getBrand() + " car...");
- }
- private String getBrand() {
- return "Benz";
- }
- }
- //寶馬汽車
- public class BaoMaCar extends AbstractCar {
- public BaoMaCar(IEngine engine) {
- super(engine);
- }
- @Override
- public void drive() {
- this.engine.start();
- System.out.println("Drive " + getBrand() + " car...");
- }
- private String getBrand() {
- return "BMW";
- }
- }
細心的小伙伴,可以發現不同子類BaoMaCar和BenzCar的drive方法,還是有相同代碼,所以我們可以再擴展一個抽象子類,把drive方法推進去,如下:
- public abstract class AbstractRefinedCar extends AbstractCar {
- public AbstractRefinedCar(IEngine engine) {
- super(engine);
- }
- @Override
- public void drive() {
- this.engine.start();
- System.out.println("Drive " + getBrand() + " car...");
- }
- abstract String getBrand();
- }
- //寶馬
- public class BaoMaRefinedCar extends AbstractRefinedCar {
- public BaoMaRefinedCar(IEngine engine) {
- super(engine);
- }
- @Override
- String getBrand() {
- return "BMW";
- }
- }
如果再添加一個新品牌,搞個子類,繼承AbstractRefinedCar即可,如果新增一種啟動引擎,也是搞個類實現IEngine接口即可
6. Shotgun Surgery(散彈式修改)
當你實現某個小功能時,你需要在很多不同的類做出小修改。這就是Shotgun Surgery(散彈式修改)。它跟發散式變化(Divergent Change) 的區別就是,它指的是同時對多個類進行單一的修改,發散式變化指在一個類中修改多處。反例如下:
- public class DbAUtils {
- @Value("${db.mysql.url}")
- private String mysqlDbUrl;
- ...
- }
- public class DbBUtils {
- @Value("${db.mysql.url}")
- private String mysqlDbUrl;
- ...
- }
多個類使用了db.mysql.url這個變量,如果將來需要切換mysql到別的數據庫,如Oracle,那就需要修改多個類的這個變量!
如何優化呢?將各個修改點,集中到一起,抽象成一個新類。
★ 可以使用 Move Method (搬移函數)和 Move Field (搬移字段)把所有需要修改的代碼放進同一個類,如果沒有合適的類,就去new一個。”
正例如下:
- public class DbUtils {
- @Value("${db.mysql.url}")
- private String mysqlDbUrl;
- ...
- }
7. Feature Envy (依戀情節)
某個函數為了計算某個值,從另一個對象那里調用幾乎半打的取值函數。通俗點講,就是一個函數使用了大量其他類的成員,有人稱之為紅杏出墻的函數。反例如下:
- public class User{
- private Phone phone;
- public User(Phone phone){
- this.phone = phone;
- }
- public void getFullPhoneNumber(Phone phone){
- System.out.println("areaCode:" + phone.getAreaCode());
- System.out.println("prefix:" + phone.getPrefix());
- System.out.println("number:" + phone.getNumber());
- }
- }
如何解決呢?在這種情況下,你可以考慮將這個方法移動到它使用的那個類中。例如,要將 getFullPhoneNumber()從 User 類移動到Phone類中,因為它調用了Phone類的很多方法。
8. Data Clumps(數據泥團)
數據項就像小孩子,喜歡成群結隊地呆在一塊。如果一些數據項總是一起出現的,并且一起出現更有意義的,就可以考慮,按數據的業務含義來封裝成數據對象。反例如下:
- public class User {
- private String firstName;
- private String lastName;
- private String province;
- private String city;
- private String area;
- private String street;
- }
正例:
- public class User {
- private UserName username;
- private Adress adress;
- }
- class UserName{
- private String firstName;
- private String lastName;
- }
- class Address{
- private String province;
- private String city;
- private String area;
- private String street;
- }
9. Primitive Obsession (基本類型偏執)
多數編程環境都有兩種數據類型,結構類型和基本類型。這里的基本類型,如果指Java語言的話,不僅僅包括那八大基本類型哈,也包括String等。如果是經常一起出現的基本類型,可以考慮把它們封裝成對象。我個人覺得它有點像Data Clumps(數據泥團) 舉個反例如下:
- // 訂單
- public class Order {
- private String customName;
- private String address;
- private Integer orderId;
- private Integer price;
- }
正例:
- // 訂單類
- public class Order {
- private Custom custom;
- private Integer orderId;
- private Integer price;
- }
- // 把custom相關字段封裝起來,在Order中引用Custom對象
- public class Custom {
- private String name;
- private String address;
- }
當然,這里不是所有的基本類型,都建議封裝成對象,有關聯或者一起出現的,才這么建議哈。
10. Switch Statements (Switch 語句)
這里的Switch語句,不僅包括Switch相關的語句,也包括多層if...else的語句哈。很多時候,switch語句的問題就在于重復,如果你為它添加一個新的case語句,就必須找到所有的switch語句并且修改它們。
示例代碼如下:
- String medalType = "guest";
- if ("guest".equals(medalType)) {
- System.out.println("嘉賓勛章");
- } else if ("vip".equals(medalType)) {
- System.out.println("會員勛章");
- } else if ("guard".equals(medalType)) {
- System.out.println("守護勛章");
- }
- ...
這種場景可以考慮使用多態優化:
- //勛章接口
- public interface IMedalService {
- void showMedal();
- }
- //守護勛章策略實現類
- public class GuardMedalServiceImpl implements IMedalService {
- @Override
- public void showMedal() {
- System.out.println("展示守護勛章");
- }
- }
- //嘉賓勛章策略實現類
- public class GuestMedalServiceImpl implements IMedalService {
- @Override
- public void showMedal() {
- System.out.println("嘉賓勛章");
- }
- }
- //勛章服務工廠類
- public class MedalServicesFactory {
- private static final Map<String, IMedalService> map = new HashMap<>();
- static {
- map.put("guard", new GuardMedalServiceImpl());
- map.put("vip", new VipMedalServiceImpl());
- map.put("guest", new GuestMedalServiceImpl());
- }
- public static IMedalService getMedalService(String medalType) {
- return map.get(medalType);
- }
- }
當然,多態只是優化的一個方案,一個方向。如果只是單一函數有些簡單選擇示例,并不建議動不動就使用動態,因為顯得有點殺雞使用牛刀了。
11.Parallel Inheritance Hierarchies( 平行繼承體系)
平行繼承體系 其實算是Shotgun Surgery的特殊情況啦。當你為A類的一個子類Ax,也必須為另一個類B相應的增加一個子類Bx。
解決方法:遇到這種情況,就要消除兩個繼承體系之間的引用,有一個類是可以去掉繼承關系的。
12. Lazy Class (冗贅類)
把這些不再重要的類里面的邏輯,合并到相關類,刪掉舊的。一個比較常見的場景就是,假設系統已經有日期工具類DateUtils,有些小伙伴在開發中,需要用到日期轉化等,不管三七二十一,又自己實現一個新的日期工具類。
13. Speculative Generality(夸夸其談未來性)
盡量避免過度設計的代碼。例如:
只有一個if else,那就不需要班門弄斧使用多態;
如果某個抽象類沒有什么太大的作用,就運用Collapse Hierarchy(折疊繼承體系)
如果函數的某些參數沒用上,就移除。
14. Temporary Field(令人迷惑的臨時字段)
某個實例變量僅為某種特定情況而定而設,這樣的代碼就讓人不易理解,我們稱之為 Temporary Field(令人迷惑的臨時字段)。反例如下:
- public class PhoneAccount {
- private double excessMinutesCharge;
- private static final double RATE = 8.0;
- public double computeBill(int minutesUsed, int includedMinutes) {
- excessMinutesCharge = 0.0;
- int excessMinutes = minutesUsed - includedMinutes;
- if (excessMinutes >= 1) {
- excessMinutesexcessMinutesCharge = excessMinutes * RATE;
- }
- return excessMinutesCharge;
- }
- public double chargeForExcessMinutes(int minutesUsed, int includedMinutes) {
- computeBill(minutesUsed, includedMinutes);
- return excessMinutesCharge;
- }
- }
思考一下,臨時字段excessMinutesCharge是否多余呢?
15. Message Chains (過度耦合的消息鏈)
當你看到用戶向一個對象請求另一個對象,然后再向后者請求另一個對象,然后再請求另一個對象...這就是消息鏈。實際代碼中,你看到的可能是一長串getThis()或一長串臨時變量。反例如下:
- A.getB().getC().getD().getTianLuoBoy().getData();
A想要獲取需要的數據時,必須要知道B,又必須知道C,又必須知道D...其實A需要知道得太多啦,回頭想下封裝性,嘻嘻。其實可以通過拆函數或者移動函數解決,比如由B作為代理,搞個函數直接返回A需要數據。
16. Middle Man (中間人)
對象的基本特征之一就是封裝,即對外部世界隱藏其內部細節。封裝往往伴隨委托,過度運用委托就不好:某個類接口有一半的函數都委托給其他類。可以使用Remove Middle Man優化。反例如下:
- A.B.getC(){
- return C.getC();
- }
其實,A可以直接通過C去獲取C,而不需要通過B去獲取。
17. Inappropriate Intimacy(狎昵關系)
如果兩個類過于親密,過分狎昵,你中有我,我中有你,兩個類彼此使用對方的私有的東西,就是一種壞代碼味道。我們稱之為Inappropriate Intimacy(狎昵關系)
建議盡量把有關聯的方法或屬性抽離出來,放到公共類,以減少關聯。
18. Alternative Classes with Different Interfaces (異曲同工的類)
A類的接口a,和B類的接口b,做的的是相同一件事,或者類似的事情。我們就把A和B叫做異曲同工的類。
可以通過重命名,移動函數,或抽象子類等方式優化
19. Incomplete Library Class (不完美的類庫)
大多數對象只要夠用就好,如果類庫構造得不夠好,我們不可能修改其中的類使它完成我們希望完成的工作。可以醬紫:包一層函數或包成新的類。
20. Data Class (純數據類)
什么是Data Class? 它們擁有一些字段,以及用于訪問(讀寫)這些字段的函數。這些類很簡單,僅有公共成員變量,或簡單操作的函數。
如何優化呢?將相關操作封裝進去,減少public成員變量。比如:
如果擁有public字段-> Encapsulate Field
如果這些類內含容器類的字段,應該檢查它們是不是得到了恰當地封裝-> Encapsulate Collection封裝起來
對于不該被其他類修改的字段-> Remove Setting Method->找出取值/設置函數被其他類運用的地點-> Move Method 把這些調用行為搬移到Data Class來。如果無法搬移整個函數,就運用Extract Method產生一個可被搬移的函數->Hide Method把這些取值/設置函數隱藏起來。
21. Refused Bequest (被拒絕的饋贈)
子類應該繼承父類的數據和函數。子類繼承得到所有函數和數據,卻只使用了幾個,那就是繼承體系設計錯誤,需要優化。
需要為這個子類新建一個兄弟類->Push Down Method和Push Down Field把所有用不到的函數下推給兄弟類,這樣一來,超類就只持有所有子類共享的東西。所有超類都應該是抽象的。
如果子類復用了超類的實現,又不愿意支持超類的接口,可以不以為然。但是不能胡亂修改繼承體系->Replace Inheritance with Delegation(用委派替換繼承).
22. Comments (過多的注釋)
這個點不是說代碼不建議寫注釋哦,而是,建議大家避免用注釋解釋代碼,避免過多的注釋。這些都是常見注釋的壞味道:
多余的解釋
日志式注釋
用注釋解釋變量等
...
如何優化呢?
方法函數、變量的命名要規范、淺顯易懂、避免用注釋解釋代碼。
關鍵、復雜的業務,使用清晰、簡明的注釋
23. 神奇命名
方法函數、變量、類名、模塊等,都需要簡單明了,淺顯易懂。避免靠自己主觀意識瞎起名字。
反例:
- boolean test = chenkParamResult(req);
正例:
- boolean isParamPass = chenkParamResult(req);
24. 神奇魔法數
日常開發中,經常會遇到這種代碼:
- if(userType==1){
- //doSth1
- }else If( userType ==2){
- //doSth2
- }
- ...
代碼中的這個1和2都表示什么意思呢?再比如setStatus(1)中的1又表示什么意思呢?看到類似壞代碼,可以這兩種方式優化:
新建個常量類,把一些常量放進去,統一管理,并且寫好注釋;
建一個枚舉類,把相關的魔法數字放到一起管理。
25. 混亂的代碼層次調用
我們代碼一般會分dao層、service層和controller層。
dao層主要做數據持久層的工作,與數據庫打交道。
service層主要負責業務邏輯處理。
controller層負責具體的業務模塊流程的控制。
所以一般就是controller調用service,service調dao。如果你在代碼看到controller直接調用dao,那可以考慮是否優化啦。反例如下:
- @RestController("user")
- public class UserController {
- Autowired
- private UserDao userDao;
- @RequestMapping("/queryUserInfo")
- public String queryUserInfo(String userName) {
- return userDao.selectByUserName(userName);
- }
- }