请问这段代码有什么问题？完全看不出有什么异常。。

V2EX = way to explore

V2EX 是一个关于分享和探索的地方

现在注册

已注册用户请登录

这是一个创建于 1943 天前的主题，其中的信息可能已经有所发展或是发生改变。

请问

代码

异常

完全

45 条回复 • 2018-12-25 14:39:01 +08:00

momuou

2018-12-24 15:03:34 +08:00

// 业务类型
if (StringUtils.isNotBlank(countTypeId) && !"null".equals(countTypeId)) {
paramMap.put("countTypeId", countTypeId.trim());
// 日期
if (StringUtils.isNotBlank(dealDate) && !"null".equals(dealDate)) {
paramMap.put("dealDate", dealDate.trim());
//地市编码
if (StringUtils.isNotBlank(regionCode) && !"null".equals(regionCode)) {
paramMap.put("regionCode", regionCode.trim());
if (StringUtils.isNotBlank(payType) && !"null".equals(payType)) {
//-支付类型（ 0 待确认 1 通过 2 不通过
paramMap.put("payType", payType.trim());
try {
int i = dataRecountService.updateDataPayType(paramMap);
if (i > 0) {
result.put("status", "1");
result.put("msg", "提交成功");
}
} catch (Exception e) {
logger.error(e.getMessage(), e);
result.put("msg", "网络异常，请稍后再尝试。");
}
} else {
result.put("msg", "选择的支付类型有误，请稍后重试");
}
} else {
result.put("msg", "请选择确切地市进行查询后再进行审核");
}
} else {
result.put("msg", "请选择确切日期进行查询后再进行审核.");
}
} else {
result.put("msg", "业务类型不明，请联系管理员");
}

he583899772

2018-12-24 15:06:50 +08:00

一大推 if else 看到就头痛

wsyzzz

2018-12-24 15:08:52 +08:00

跑一下看看报错呗，，看文本调试也太头疼了

jingyulong

2018-12-24 15:10:17 +08:00

现在哪里有报错吗？

ukipoi

2018-12-24 15:13:25 +08:00

countTypeId = " null";
这样子是不是就不符合要求了

Vegetable

2018-12-24 15:14:16 +08:00

不是很合理哦,你上边是个 if,是不是"null "都能过?
本身这个数据是哪来的我不要清楚,不过你是不是应该在 trim 后判空?

Vegetable

2018-12-24 15:15:03 +08:00

@Vegetable *要=>是

jcsmallming

2018-12-24 15:15:27 +08:00

是有报错还是运行结果不符合预期？你直接贴这一段代码上来没有补充说明真的很难看啊

misaka19000

2018-12-24 15:19:05 +08:00

这个代码写的好烂，建议先重构下

jorneyr

2018-12-24 15:23:14 +08:00

这种数据校验, 我们会选择使用 hibernate validator 使用 @Valid 自动处理, 写代码太过麻烦

nananqujava

2018-12-24 15:25:02 +08:00

我靠又是这种箭头型代码，这种代码也能通过审核？

pipicat

2018-12-24 15:47:31 +08:00

每次校验的方法 StringUtils.isNotBlank(value) && !"null".equals(value) 应该可以合并成一个函数吧。而且代码都是有规律的，可以用反射字段名统一添加吧，搜了一下 @jorneyr 说的 hibernate validator 应该可以，原理应该类似

kyuuseiryuu

2018-12-24 15:47:36 +08:00

这个令人头痛的代码本身就是“异常”

momuou

2018-12-24 16:06:38 +08:00

@wsyzzz 不是报错--是想优化一下这种写法。。。

momuou

2018-12-24 16:07:51 +08:00

@jcsmallming 这种写法肯定是不符合预期的，其实一直都是写 if()else{return}这种的，可是参数多了就觉得麻烦，而且最后还是会走到事务里面去.

momuou

2018-12-24 16:09:25 +08:00

@misaka19000 这本身就是一段烂代码，就是想请教一下你怎么重构，瞎子都知道这种代码难维护，可是你要怎么写才能符合这种写法又优雅呢？

momuou

2018-12-24 16:10:09 +08:00

@kyuuseiryuu 就是本着我写着爽，维护的人想砍我的心态。。所以才放上来的。

momuou

2018-12-24 16:11:00 +08:00

@jorneyr 谢谢，我去查询一下。

james2013

2018-12-24 16:14:42 +08:00

问题:
1)判断为空完善,应改为!"null".equals(countTypeId.trim())
2)if else 嵌套太深,如下减少层级到 1 层:
if (!StringUtils.isNotBlank(countTypeId)) {
return;
}
if (!StringUtils.isNotBlank(dealDate)) {
return;
}
...

momuou

2018-12-24 16:20:04 +08:00

好吧，大背景就是本来写了一段自我感觉良好的代码，可是硬盘丢失了，代码只能重写，却写出这种后人难以维护的代码，感觉不太好，所以发上来请教一下各位。。刚刚上了个厕所，好像又记起来之前怎么写的了。
if (StringUtils.isBlank(countTypeId) || "null".equals(countTypeId)) {
// 业务类型
result.put("msg", "业务类型不明，请联系管理员");
}
else if(StringUtils.isBlank(dealDate) || "null".equals(dealDate)){
// 日期
result.put("msg", "请选择确切日期进行查询后再进行审核.");
}else if(StringUtils.isBlank(regionCode) || "null".equals(regionCode)){
//地市编码
result.put("msg", "请选择确切地市进行查询后再进行审核");
}else if(StringUtils.isBlank(payType) || "null".equals(payType)){
//-支付类型（ 0 待确认 1 通过 2 不通过
result.put("msg", "选择的支付类型有误，请稍后重试");
}else{
paramMap.put("countTypeId", countTypeId.trim());
paramMap.put("dealDate", dealDate.trim());
paramMap.put("regionCode", regionCode.trim());
paramMap.put("payType", payType.trim());
int i = dataRecountService.updateDataPayType(paramMap);
if (i > 0) {
result.put("status", "1");
result.put("msg", "提交成功");
}
}
return result;

annielong

2018-12-24 16:21:02 +08:00

反正个人写条件两种用 if，三种以上就换 switch，不过返回值提示这种可以定义字典，或者数组，可以统一维护

momuou

2018-12-24 16:21:55 +08:00

@james2013 这种写一两个还好，写多了容易烦躁

momuou

2018-12-24 16:22:57 +08:00

不会贴图，所以没什么贴上来没什么格式。。感谢提建议的各位。

pipicat

2018-12-24 18:24:29 +08:00

写了一段，供你参考。
public class Test {

class Model {
@Warning("业务类型不明，请联系管理员")
public String countTypeId;
@Warning("请选择确切日期进行查询后再进行审核")
public String dealDate;
@Warning("请选择确切地市进行查询后再进行审核")
public String regionCode;
@Warning("选择的支付类型有误，请稍后重试")
public String payType;
}

public void Test(){

Model model = new Model();
HashMap<String,String> result = new HashMap<>();
HashMap<String,String> paramMap = new HashMap<>();

Field[] declaredFields = model.getClass().getDeclaredFields();
boolean pass = true;

for (Field field:declaredFields){
if (field.getAnnotation(Warning.class)!=null){
field.setAccessible(true);
try {
Object o = field.get(model);

if (!isVaild(String.valueOf(o))) {
pass = false;
result.put(field.getName(),field.getAnnotation(Warning.class).value());
break;
} else {
paramMap.put(field.getName(),String.valueOf(o).trim());
}
} catch (IllegalAccessException e) {
e.printStackTrace();
}
}
}

if (pass) {
try {
int i = dataRecountService.updateDataPayType(paramMap);
if (i > 0) {
result.put("status", "1");
result.put("msg", "提交成功");
}
} catch (Exception e) {
logger.error(e.getMessage(), e);
result.put("msg", "网络异常，请稍后再尝试。");
}
} else {

}

}

public boolean isVaild(String str){
return StringUtils.isBlank(str) && !"null".equals(str);
}

}

@Target(FIELD)
@Retention(RUNTIME)
@interface Warning{
String value() default "";
}

具体细节可能还要调一下。

lihongjie0209

2018-12-24 18:38:26 +08:00

lihongjie0209

2018-12-24 19:02:12 +08:00

![]( http://liimg.oss-cn-shenzhen.aliyuncs.com/18-12-24/67413298.jpg)

lihongjie0209

2018-12-24 19:07:29 +08:00

https://imgur.com/a/zlgkDKP

laoyur

2018-12-24 19:07:36 +08:00

看到这样排版还肯帮忙的，是真的热心肠了

lihongjie0209

2018-12-24 19:10:03 +08:00

@laoyur 主要是想试一下 gif 录制工具还有在 v2 回复图片, 后者看来失败了

devilyaos

2018-12-24 21:28:18 +08:00 via iPhone

counttypeid 为 null （有空格）会进下面的逻辑吧？此时应该条件匹配不上？或者 null 是大写，大小写匹配可以用 equalsignore 嘛……

iceheart

2018-12-25 08:42:27 +08:00 via Android

//抛块砖头，帮你重构了一下，java 语法不太熟，将就着写吧
//重复的动作首先要封装一下
bool check(string typeName string typeValue) {
if (StringUtils.isBlank(typeValue) || "null".equals(typeValue)) {
return false;
}
paramMap.put(typeName, typeValue.trim());
return true;
}
...
//try catch 可以简化分支处理
try {
if (!check(countTypeId)) throw Exception("your message");
if (!check(dealDate)) throw Exception("your message");
if (!check(regionCode)) throw Exception("your message");
if (!check(payType)) throw Exception("your message");
int i = 0;
try {
i = dataRecountService.updateDataPayType(paramMap);
}catch (Exception e) {
logger.error(e.getMessage(), e);
throw Exception("网络异常，请稍后再尝试。" );
}
if (i > 0) {
result.put("status", "1");
result.put("msg", "提交成功");
}
}cache(Exception e) {
result.put(e.getMessage());
}

momuou

2018-12-25 09:14:32 +08:00

@pipicat 谢谢

momuou

2018-12-25 09:14:44 +08:00

@lihongjie0209 谢谢

momuou

2018-12-25 09:16:06 +08:00

@devilyaos 其实判断不是重点。。重点就是怎么优化这种多个判断的，不过还是很感谢

momuou

2018-12-25 09:18:00 +08:00

@iceheart 谢谢，抛异常这种考虑过，不过也是感觉太繁琐

tingfang

2018-12-25 09:34:51 +08:00

这种 ifelse 建议使用卫语句。

tingfang

2018-12-25 09:36:31 +08:00

if(!conditionA){
return;
}
if(!conditionB){
return;
}
if(!conditionC){
return;
}
...
return;

icop

2018-12-25 09:43:54 +08:00

debug 不行吗？ IDEA 的 DEBUG 啊。

aa514758835

2018-12-25 09:56:59 +08:00

这是 java 吗~ 我看不懂

wuchenl

2018-12-25 10:12:35 +08:00

使用 equals 的时候，记得考虑空格的情况下。即.trim()。
然后针对这种多个参数要判断是否为空的情况，建议弄一个 BO，接收。然后上面加注解等进行校验。这种可读性太差了。
有 @NotNull，@NotEmpty 注解，搭配 message 属性使用即可；

@NotEmpty(message="证件号不能为空！")
private String certNo;

具体使用时 Controller 层使用 @Valid 注解也可，方法内部进行校验也可。详细的可以自己了解下哦=0=
public String test(@Valid User user){

}

zhaogaz

2018-12-25 10:44:15 +08:00

你把以上内容封装成一个对象，然后用 valid 验证的框架去做验证。验证内容都注解写就好了。。。

momuou

2018-12-25 14:35:50 +08:00

@tingfang 谢谢，不过这种写法我觉得太繁琐了，可以看看我 20L 写的最新的。也是感觉写的最方便的写法了

momuou

2018-12-25 14:36:15 +08:00

@icop 这不是执行问题，是优化问题。

momuou

2018-12-25 14:37:52 +08:00

@wuchenl 嗯，这位老哥的方法跟 24L 的有点相似，有空余时间我也要学习一下这种利用注解的方式.谢谢

momuou

2018-12-25 14:39:01 +08:00

@zhaogaz 嗯，楼上多次提及 vaild 校验，这个更多我在前端接触过，服务端的真要学习一下