25种代码坏味道总结+优化示例

什么样的代码是好代码呢？好的代码应该命名规范、可读性强、扩展性强、健壮性......而不好的代码又有哪些典型特征呢？这25种代码坏味道大家要注意啦

• 公众号：捡田螺的小男孩
• github地址

重复代码就是不同地点，有着相同的程序结构。一般是因为需求迭代比较快，开发小伙伴担心影响已有功能，就复制粘贴造成的。重复代码很难维护的，如果你要修改其中一段的代码逻辑，就需要修改多次，很可能出现遗漏的情况。

如何优化重复代码呢？分三种情况讨论：

1. 同一个类的两个函数含有相同的表达式

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{doSomething1doSomething2 }}

1. 两个互为兄弟的子类内含相同的表达式

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 }}

1. 两个毫不相关的类出现重复代码

如果是两个毫不相关的类出现重复代码，可以使用Extract Class将重复代码提炼到一个类中。这个新类可以是一个普通类，也可以是一个工具类，看具体业务怎么划分吧。

长函数是指一个函数方法几百行甚至上千行，可读性大大降低，不便于理解。反例如下：

public class Test { private String name; private Vector orders = new Vector; public void printOwing { //print banner System.out.println; System.out.println; System.out.println; //calculate totalAmount Enumeration env = orders.elements; double totalAmount = 0.0; while ) { Order order = env.nextElement; totalAmount += order.getAmout; } //print details System.out.println; System.out.println; ...... }}

可以使用Extract Method，抽取功能单一的代码段，组成命名清晰的小函数，去解决长函数问题，正例如下:

public class Test { private String name; private Vector orders = new Vector; public void printOwing { //print banner printBanner; //calculate totalAmount double totalAmount = getTotalAmount; //print details printDetail; } void printBanner{ System.out.println; System.out.println; System.out.println; } double getTotalAmount{ Enumeration env = orders.elements; double totalAmount = 0.0; while ) { Order order = env.nextElement; totalAmount += order.getAmout; } return totalAmount; } void printDetail{ System.out.println; System.out.println; } }

一个类做太多事情，维护了太多功能，可读性变差，性能也会下降。举个例子，订单相关的功能你放到一个类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; } }}

方法参数数量过多的话，可读性很差。如果有多个重载方法，参数很多的话，有时候你都不知道调哪个呢。并且，如果参数很多，做新老接口兼容处理也比较麻烦。

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;}

对程序进行维护时, 如果添加修改组件, 要同时修改一个类中的多个方法, 那么这就是 Divergent Change。举个汽车的例子，某个汽车厂商生产三种品牌的汽车：BMW、Benz和LaoSiLaiSi，每种品牌又可以选择燃油、纯电和混合动力。反例如下：

public class Car { private String name; void start { if )) { System.out.println; } else if )) { System.out.println; } else if )) { System.out.println; } } void drive { this.start; System.out.println + " car..."); } String getBrand { if )) { return "BMW"; } else if )) { return "Benz"; } else if )) { return "LaoSiLaiSi"; } return null; } }

如果新增一种品牌新能源电车，然后它的启动引擎是核动力呢，那么就需要修改Car类的start和getBrand方法啦，这就是代码坏味道：Divergent Change （发散式变化）。

如何优化呢？一句话总结：拆分类，将总是一起变化的东西放到一块。

★

• 运用提炼类 拆分类的行为。
• 如果不同的类有相同的行为，提炼超类 和 提炼子类。

”

正例如下：

因为Engine是独立变化的，所以提取一个Engine接口，如果新加一个启动引擎，多一个实现类即可。如下：

//IEnginepublic interface IEngine { void start;}public class HybridEngineImpl implements IEngine { @Override public void start { System.out.println; }}

因为drive方法依赖于Car，IEngine，getBand方法;getBand方法是变化的，也跟Car是有关联的，所以可以搞个抽象Car的类，每个品牌汽车继承于它即可，如下

public abstract class AbstractCar { protected IEngine engine; public AbstractCar { this.engine = engine; } public abstract void drive;}//奔驰汽车public class BenzCar extends AbstractCar { public BenzCar { super; } @Override public void drive { this.engine.start; System.out.println + " car..."); } private String getBrand { return "Benz"; }}//宝马汽车public class BaoMaCar extends AbstractCar { public BaoMaCar { super; } @Override public void drive { this.engine.start; System.out.println + " car..."); } private String getBrand { return "BMW"; }}

细心的小伙伴，可以发现不同子类BaoMaCar和BenzCar的drive方法，还是有相同代码，所以我们可以再扩展一个抽象子类，把drive方法推进去，如下:

public abstract class AbstractRefinedCar extends AbstractCar { public AbstractRefinedCar { super; } @Override public void drive { this.engine.start; System.out.println + " car..."); } abstract String getBrand;}//宝马public class BaoMaRefinedCar extends AbstractRefinedCar { public BaoMaRefinedCar { super; } @Override String getBrand { return "BMW"; }}

如果再添加一个新品牌，搞个子类，继承AbstractRefinedCar即可，如果新增一种启动引擎，也是搞个类实现IEngine接口即可

当你实现某个小功能时，你需要在很多不同的类做出小修改。这就是Shotgun Surgery（散弹式修改）。它跟发散式变化 的区别就是，它指的是同时对多个类进行单一的修改，发散式变化指在一个类中修改多处。反例如下：

public class DbAUtils { @Value private String mysqlDbUrl; ...}public class DbBUtils { @Value private String mysqlDbUrl; ...}

多个类使用了db.mysql.url这个变量，如果将来需要切换mysql到别的数据库，如Oracle，那就需要修改多个类的这个变量！

如何优化呢？将各个修改点，集中到一起，抽象成一个新类。

★ 可以使用 Move Method （搬移函数）和 Move Field （搬移字段）把所有需要修改的代码放进同一个类，如果没有合适的类，就去new一个。
”

正例如下：

public class DbUtils { @Value private String mysqlDbUrl; ...}

某个函数为了计算某个值，从另一个对象那里调用几乎半打的取值函数。通俗点讲，就是一个函数使用了大量其他类的成员，有人称之为红杏出墙的函数。反例如下：

public class User{ private Phone phone; public User{ this.phone = phone; } public void getFullPhoneNumber{ System.out.println); System.out.println); System.out.println); }}

如何解决呢？在这种情况下，你可以考虑将这个方法移动到它使用的那个类中。例如，要将 getFullPhoneNumber从 User 类移动到Phone类中，因为它调用了Phone类的很多方法。

数据项就像小孩子，喜欢成群结队地呆在一块。如果一些数据项总是一起出现的，并且一起出现更有意义的，就可以考虑，按数据的业务含义来封装成数据对象。反例如下：

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;}

多数编程环境都有两种数据类型，结构类型和基本类型。这里的基本类型，如果指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;}

当然，这里不是所有的基本类型，都建议封装成对象，有关联或者一起出现的，才这么建议哈。

这里的Switch语句，不仅包括Switch相关的语句，也包括多层if...else的语句哈。很多时候，switch语句的问题就在于重复，如果你为它添加一个新的case语句，就必须找到所有的switch语句并且修改它们。

示例代码如下：

String medalType = "guest"; if ) { System.out.println; } else if ) { System.out.println; } else if ) { 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 map = new HashMap<>; static { map.put); map.put); map.put); } public static IMedalService getMedalService { return map.get; }}

当然，多态只是优化的一个方案，一个方向。如果只是单一函数有些简单选择示例，并不建议动不动就使用动态，因为显得有点杀鸡使用牛刀了。

平行继承体系 其实算是Shotgun Surgery的特殊情况啦。当你为A类的一个子类Ax，也必须为另一个类B相应的增加一个子类Bx。

建议尽量把有关联的方法或属性抽离出来，放到公共类，以减少关联。

可以通过重命名，移动函数，或抽象子类等方式优化

大多数对象只要够用就好，如果类库构造得不够好，我们不可能修改其中的类使它完成我们希望完成的工作。可以酱紫：包一层函数或包成新的类。

什么是Data Class 它们拥有一些字段，以及用于访问这些字段的函数。这些类很简单，仅有公共成员变量，或简单操作的函数。

如何优化呢？将相关操作封装进去，减少public成员变量。比如：

• 如果拥有public字段-> Encapsulate Field
• 如果这些类内含容器类的字段，应该检查它们是不是得到了恰当地封装-> Encapsulate Collection封装起来
• 对于不该被其他类修改的字段-> Remove Setting Method->找出取值/设置函数被其他类运用的地点-> Move Method 把这些调用行为搬移到Data Class来。如果无法搬移整个函数，就运用Extract Method产生一个可被搬移的函数->Hide Method把这些取值/设置函数隐藏起来。

子类应该继承父类的数据和函数。子类继承得到所有函数和数据，却只使用了几个，那就是继承体系设计错误，需要优化。

• 需要为这个子类新建一个兄弟类->Push Down Method和Push Down Field把所有用不到的函数下推给兄弟类，这样一来，超类就只持有所有子类共享的东西。所有超类都应该是抽象的。
• 如果子类复用了超类的实现，又不愿意支持超类的接口，可以不以为然。但是不能胡乱修改继承体系->Replace Inheritance with Delegation.

这个点不是说代码不建议写注释哦，而是，建议大家避免用注释解释代码，避免过多的注释。这些都是常见注释的坏味道：

• 多余的解释
• 日志式注释
• 用注释解释变量等
• ...

如何优化呢？

• 方法函数、变量的命名要规范、浅显易懂、避免用注释解释代码。
• 关键、复杂的业务，使用清晰、简明的注释

方法函数、变量、类名、模块等，都需要简单明了，浅显易懂。避免靠自己主观意识瞎起名字。

反例：

boolean test = chenkParamResult;

正例：

boolean isParamPass = chenkParamResult;

日常开发中，经常会遇到这种代码：

if{ //doSth1}else If{ //doStp}...

代码中的这个1和2都表示什么意思呢？再比如setStatus中的1又表示什么意思呢？看到类似坏代码，可以这两种方式优化：

• 新建个常量类，把一些常量放进去，统一管理，并且写好注释；
• 建一个枚举类，把相关的魔法数字放到一起管理。

我们代码一般会分dao层、service层和controller层。

• dao层主要做数据持久层的工作，与数据库打交道。
• service层主要负责业务逻辑处理。
• controller层负责具体的业务模块流程的控制。

所以一般就是controller调用service，service调dao。如果你在代码看到controller直接调用dao，那可以考虑是否优化啦。反例如下：

@RestControllerpublic class UserController { Autowired private UserDao userDao; @RequestMapping public String queryUserInfo { return userDao.selectByUserName; }}

• 软工实验：常见的代码坏味道以及重构举例
• 22种代码的坏味道，一句话概括
• 【重构】 代码的坏味道总结 Bad Smell
• Code Smell
• 《重构改善既有代码的设计》