本文有赵玉开老师总结整理,经本人同意转载至此。如需转载,请务必保留出处!
本文作者:赵玉开,十年以上互联网研发经验,2013年加入京东,在运营研发部任架构师,期间先后主持了物流系统自动化运维平台、青龙数据监控系统和物流开放平台的研发工作,具有丰富的物流系统业务和架构经验。在此之前在和讯网负责股票基金行情系统的研发工作,具备高并发、高可用互联网应用研发经验。
贾言 | |
验幻空越重, 命循频异长。 依轮线日简, 接偶正分壮。 | 言欢空月虫, 明勋品宜昌。 依伦先日贱, 洁偶正粉妆。 |
贾言
架构师说, 用20个字描述代码评审的内容, 自省也省人。由于是一字一含义, 不连贯, 为了增强趣味性, 每句都增加对应的歪解。只是对常见评审的描述, 不尽之处,欢迎补充!
验幻空越重 -- 言欢空月虫
验: 公共方法都要做参数的校验, 参数校验不通过明确抛出异常或对应响应码
幻: 在代码中要杜绝幻数, 幻数可定义为枚举或常量以增强其可读性
空: 要时刻警惕空指针异常, 常见的 a.equals(b) 要把常量放到左侧, aInteger == 10 如果 aInteger 为空时会抛出空指针异常
越: 如果方法传入数组下标作为参数, 要在一开始就做下标越界的校验, 避免下标越界异常
重: 不要写重复代码, 重复代码要使用重构工具提取重构
命循频异长 - 明勋品宜昌
命: 包 / 类 / 方法 / 字段 / 变量 / 常量的命名要遵循规范, 要名副其实, 这不但可以增加可读性, 还可以在起名的过程中引导我们思考方法 / 变量 / 类的职责是否合适
循: 不要在循环中调用服务,不要在循环中做数据库等跨网络操作
频: 写每一个方法时都要知道这个方法的调用频率, 一天多少, 一分多少, 一秒多少,峰值可能达到多少, 调用频率高的一定要考虑性能指标, 考虑是否会打垮数据库, 是否会击穿缓存
异: 异常处理是程序员最基本的素质, 不要处处捕获异常, 对于捕获了只写日志, 没有任何处理的 catch 要问一问自己, 这样吃掉异常, 是否合理
下面是一个反例, 在导出文件的controller方法中做了两层的try...catch, 在catch块中记录日志后什么都没做, 这样用户看不到真正想要的内容, 研发也只有看日志才能发现错误, 而“看日志”, 通常只有业务方反馈问题时才会看, 就会导致研发人员发现错误会比现场人员还会晚。
@RequestMapping
(value =
"/export"
)
public
void
export(CityRelationDomain condition, HttpServletResponse response) {
ZipOutputStream zos =
null
;
BufferedWriter bufferedWriter =
null
;
try
{
condition.setStart(
0
);
condition.setSize(MAX_EXPORT_LINES);
List<CityRelationDomain> list = cityRelationService.getOrdersByCondition(condition);
response.setCharacterEncoding(
"GBK"
);
response.setContentType(
"multipart/form-data"
);
response.setHeader(
"Content-Disposition"
,
"attachment;fileName=export.zip"
);
zos =
new
ZipOutputStream(response.getOutputStream());
bufferedWriter =
new
BufferedWriter(
new
OutputStreamWriter(zos,
"GBK"
));
bufferedWriter.write(
"订单类型编码,始发城市-省,始发城市-市,目的城市-省,目的城市-市"
);
ZipEntry zipEntry =
new
ZipEntry(
"export.csv"
);
zos.putNextEntry(zipEntry);
for
(CityRelationDomain domain : list) {
try
{
bufferedWriter.newLine();
bufferedWriter.write(CSVExportUtil.trans2CSV(domain.getOrderCode()));
bufferedWriter.write(
','
);
bufferedWriter.write(CSVExportUtil.trans2CSV(domain.getProvinceNameFrom()));
bufferedWriter.write(
','
);
bufferedWriter.write(CSVExportUtil.trans2CSV(domain.getCityNameFrom()));
bufferedWriter.write(
','
);
bufferedWriter.write(CSVExportUtil.trans2CSV(domain.getProvinceNameTo()));
bufferedWriter.write(
','
);
bufferedWriter.write(CSVExportUtil.trans2CSV(domain.getCityNameTo()));
}
catch
(Exception e) {
e.printStackTrace();
}
}
bufferedWriter.newLine();
bufferedWriter.flush();
zos.closeEntry();
bufferedWriter.close();
}
catch
(Exception e) {
e.printStackTrace();
logger.error(
"导出CSV文件异常"
);
}
finally
{
try
{
if
(zos !=
null
) {
zos.close();
}
if
(bufferedWriter !=
null
) {
bufferedWriter.close();
}
}
catch
(IOException e) {
e.printStackTrace();
}
}
}
长: 如果一行代码过长, 要分解开来; 如果一个方法过长, 要重构方法; 如果一个类过长要考虑拆分类
依轮线日简 - 依伦先日贱
依: 如果调用了外部依赖, 一定要搞清楚这个外部依赖可以提供的性能指标, 最好约定 SLA
轮: 不要重复造轮子, 如果已经有成熟类库实现了类似功能, 要优先使用成熟类库的方法, 这是因为成熟类库中的方法都经过很多人的测试验证, 通常情况下我们自己实现的质量最大等同于成熟类库的质量。
线: 要注意我们的 jsf 服务, web 应用, 消费消息的 worker 都是多线程环境, 要注意线程安全问题, 最典型的 HashMap , SimpleDateFormat , ArrayList 是非线程安全的, 另外如果使用 Spring 自动扫描服务, 那么这个服务默认是单例, 其内部成员是多个线程共享的, 如果直接用成员变量是有线程不安全的。
两个典型的错误代码片段:
1 ) 无视 SimpleDateFormat 非线程安全
@Service
public
class
AService {
private
static
final
SimpleDateFormat FORMAT =
new
SimpleDateFormat(
"yyyy-MM-dd"
);
public
void
doSomething() {
//use FORMAT
}
}
2 ) 使用 Service 成员变量
@Service
public
class
BService {
private
Pojo b;
public
void
doB() {
b = getB();
process(b);
}
}
日: 打印日志和设定合理的日志级别, 如有必要要添加 if 条件限定是否打印日志, 在日志中使用 JSON 序列化, 生成长字符串的 toString() 都要做 if 限定打印, 否则配置的日志级别没达到, 也会做大量字符串拼接, 占用很多 gc 年轻代 内存
典型错误示例:
@Service
public
class
FooService {
private
static
final
Logger LOGGER = LoggerFactory.getLogger(FooService.
class
);
public
void
doFooThing(Foo foo) {
LOGGER.debug(
"get parameter foo {}"
, JSONObject.toString(foo));
}
}
简: 尽可能保持整体设计的简洁, 方法实现的简洁, 要根据情况使用内存缓存, redis 缓存, jmq 异步处理。 这里的简需要把握好分寸。
接偶正分壮 - 洁偶正粉妆
接: 接口是用来隔离变化的, 如果一个业务有几种不同的形态, 但都有相同的处理, 那么可以定义接口来隔离业务形态的不同, 在服务调用处, 通过业务类型字段来获得不同的服务类。 而不要实现一个类, 然后在类的各个方法中都根据业务类型做 if else 或更复杂的各种判断。
典型示例:
做法 1 :
public
interface
BarService {
void
doBarThing(Bar b);
void
doBarFatherThing(Bar b);
}
public
class
BarServiceImpl implement BarService{
public
void
doBarThing(Bar b) {
if
(b.getType() == BarType.A) {
//do some logic
}
else
(b.getType() == BarType.B) {
//do some B type logic
}
//do other doBarThing logic
}
public
void
doBarFatherThing(Bar b) {
if
(b.getType() == BarType.A) {
//do some logic
}
else
(b.getType() == BarType.B) {
//do some B type logic
}
//do other doBarFatherThing logic
}
}
做法 2 :
public
interface
BarService {
void
doBarThing(Bar b);
void
doBarFatherThing(Bar b);
}
public
class
BarServiceFactory {
public
BarService getBarService(BarType type) {
// get bar service logic
}
}
//如果有公共逻辑就定义, 没有就不定义
public
class
BaseBarService implement BarService {
public
void
doBarThing(Bar b) {
//do other doBarThing logic
}
public
void
doBarFatherThing(Bar b) {
//do other doBarFatherThing logic
}
}
public
class
TypeABarService
extends
BaseBarService implement BarService {
public
void
doBarThing(Bar b) {
// doATypeThing
super
.doBarThing(b);
}
public
void
doBarFatherThing(Bar b) {
// do bar type A service
super
.doBarFatherThing(b);
//如果需要就调用, 不需要就不调用父类
}
}
偶: 认识系统之间的耦合关系, 通过同步数据来做两个系统之间的交互是一种很强的耦合关系,会使数据接收方依赖于数据发送方的数据库定义, 如果发送方想改数据结构, 必须要求下游接收方一起修改; 通过接口调用是一种常见的系统耦合关系, 接口的提供方要保证接口的可用性, 接口的调用方要考虑接口不可用时的应对方案; mq 消息是一种解耦的方法, 两个系统不存在实时的耦合关系。 但是 mq 解耦的方式不能滥用, 在同一系统内不宜过多使用 mq 消息来做异步, 要尽可能保证接口的性
能, 而不是通过 mq 防止出问题后重新消费。
正: 模块之间依赖关系要正向依赖, 不能让底层模块依赖于上层模块; 不能让数据层依赖于服务层也不能让服务层依赖于 UI 层; 也不能在模块之间形成循环依赖关系。
分: 分而治之,复杂的问题要分解成几个相对简单的问题来解决, 首先要分析出核心问题, 然后分析出核心的入参是什么, 结果是什么, 入参通过几步变化可以得出结果。
壮: 时刻注意程序的健壮性, 从两个方面实践提升健壮性:
1 ) 契约, 在设计接口时定义好协议参数, 并在实现时第一时间校验参数, 如果参数有问题, 直接返回给调用方; 如果出现异常情况, 也按异常情况约定应对策略
2) 考虑各种边界条件的输出, 比如运单号查询服务, 要考虑用户输入错误运单时怎么返回, 有边界的查询条件, 如果用户查询条件超过边界了, 应该返回什么
3) 为失败做设计, 如果出问题了有降级应对方案。