哪位高人能帮我优化一段 Java 代码,强迫症犯了。

2017-08-13 03:17:05 +08:00
 hsiunien

下面这段 if 简直不能忍, 效果就是通过字符串 rec 把 studentsList 赋值,求一个优雅点的,健壮的写法。

        String rec = ....
        if (rec != null) {
            Student[] students = convert(rec);
            if (students.length != 0) {
                studentsList = Arrays.asList(students);
            } else {
                studentsList = new ArrayList<>();
            }
        } else {
            studentsList = new ArrayList<>();
        }
        
6171 次点击
所在节点    程序员
44 条回复
little_cup
2017-08-13 10:18:35 +08:00
Optional.ofNullable(…)
.map(this::convert)
.stream()
.flatmap(Arrays::stream)
.collect(Collectors.toList());

这样?
hantsy
2017-08-13 10:28:49 +08:00
就一数据转化而已。

贴一段数据库初始化的代码,主要是用 Spring Data.

```
this.posts
.deleteAll()
.thenMany(
Flux
.just("Post one", "Post two")
.flatMap(
title -> this.posts.save(Post.builder().title(title).content("content of " + title).build())
)
)
.log()
.subscribe(
null,
null,
() -> log.info("done initialization...")
);
```
echotpq
2017-08-13 10:38:41 +08:00
楼主应该是处女座的
msg7086
2017-08-13 11:38:14 +08:00
if (studentsList == null)
studentsList = new ArrayList<>();
inisun
2017-08-13 11:46:33 +08:00
如果不是整个项目组都会用函数式编程,写 lambda 会被人唾弃吧。
weakish
2017-08-13 11:56:10 +08:00
`String rec`和`convert`不明确。`rec` 必然是有格式的。`convert`肯定不能转换任意字符串吧。
所以 convert 实际做的是 parse 的活。

如果`rec`包含的信息比较复杂,优先考虑用 JSON、CSV 之类通用格式,而不是特定的格式。
因为要健壮,parser 肯定要处理校验,包括输出良好的报错,方便快速定位格式错误的地方。
用通用格式,可以直接用现成的 parser (别人的代码,或者是自己积累的历史代码)。

如果`rec`包含的信息非常简单,而且能保证不出现格式错误(比如 rec 本身就是通过程序生成的),
那也可以自己写 parse 函数。
但是`rec`的格式需要注明,
或者是用长变量名,比如`rec`改成`spaceSeparatedNames`,
或者是写注释。

`convert`同样写长,比如 `parseStudents`.
然后 `parseStudents` 直接返回 List 或 ArrayList
( Arrays.asList 和 new ArrayList 返回的类型是不同的,根据需求统一到一种),
包括处理 students 为空的代码都是放到 `parseStudents` 函数里。

另外,`parseStudents`一般拒绝 null,比如用 IntelliJ 的话,参数加 @NotNull
同理,`rec`如果没有特别的理由,也拒绝 null。

如果 IDE 不支持,写个检测 Object 是否 Null 的方法,`parseStudents` 先用这个函数检测参数,
是 null 就报错,要啰嗦一点,效果也是一样的。
mtus
2017-08-13 15:12:10 +08:00
@padeoe 在 java 8 中, `Arrays.asList()` 是通过 `new ArrayList<>()` 实现的, 所以是可变的. 而 `Collections.EMPTY_LIST, Collections.emptyList(), Collections.singletonList()` 是不可变的.

```java
@SafeVarargs
@SuppressWarnings("varargs")
public static <T> List<T> asList(T... a) {
return new ArrayList<>(a);
}
```
zhx1991
2017-08-13 18:14:44 +08:00
尽量提早 return, 减少 if 的层数是增加可读性的关键

在外面 new 一个 list

然后第一 if 要是 null 直接返回

里面的判断也是一样的

如果 students 是空, 直接返回

最后一步才是赋值操作

这样最后的代码只有一层 if, 至少看起来舒服很多
sudit
2017-08-14 00:35:25 +08:00
studentList = (rec == null)?new ArrayList<>() :
Stream.of(rec.split(separator))
.map(Student::convert)
.collect(Collectors.toList());
每个人都有对优雅的理解吧
kyuuseiryuu
2017-08-14 01:11:47 +08:00
```java
String rec = ...;
do {
if (null === rec) {
studentsList = new ArrayList<>();
break;
}
Student[] students = convert(rec);
studentsList = students.length != 0 ?
Arrays.asList(students)
: new ArrayList<>();
} while (false);
```
cuebyte
2017-08-14 06:30:38 +08:00
就这破代码,这么多人说没优化空间?写 Java 写傻了吧
cuebyte
2017-08-14 06:41:13 +08:00
首先这 convert 就很蠢,我就认为你们不能改 convert 好了……

String rec = "...";
List<Student> students = convertWrapper(rec);

private List<Student> convertWrapper(String rec) {
if (rec == null) return Collections.EMPTY_LIST;

Student[] sbStudents = convert(rec);
return Arrays.asList(sbStudents);
}
BBCCBB
2017-08-14 08:42:16 +08:00
@mtus 此 ArrayList 非彼 ArrayList, 这是 Arrays 的一个内部类,是不可变的
BBCCBB
2017-08-14 08:44:14 +08:00
String rec = ....
studentsList = null;
if (rec != null) {
Student[] students = convert(rec);
if (students.length != 0) {
studentsList = Arrays.asList(students);
}
}
if(studentsList == null)
studentsList = Collections.EMPTY_LIST();
asj
2017-08-14 10:29:58 +08:00
这段代码的问题主要不在本身里面,其实你觉得不爽的地方就是空值处理。
只需要修改 convert 函数保证不返回 null 即可。
你本来的 if length != 0 就是没必要的, Arrays.asList 接受布丁不定长度参数,传入空数组是可以的。
所以基本上可以简化为
studentsList = Arrays.asList(convert(rec));
vjnjc
2017-08-14 11:49:52 +08:00
String rec = ....

if (rec != null) {
Student[] students = convert(rec);
if (students.length != 0) {
studentsList = Arrays.asList(students);
}
}
if(studentsList == null) {
studentsList = new ArrayList<>();
}

写成这样?

不过我还是喜欢这种写法,

if(XX==null){
//异常处理
}else{
//业务逻辑
}

因为一般异常处理比较简单,所以写前面。
yidinghe
2017-08-14 14:54:40 +08:00
![]( )
这应该是楼主要的了。
padeoe
2017-08-14 16:26:03 +08:00
@mtus 😂你在哪儿看到的这个说法...你看源码的吧,那是一个内部类,不是真·ArrayList,所以是个大坑。
bk201
2017-08-14 17:28:03 +08:00
@cuebyte 讲句不好听的,你这改写还是很 low 啊,个人觉得这种情况最好的还是 java stream 处理方式
liyu4
2017-08-14 17:50:05 +08:00
先处理 else

这是一个专为移动设备优化的页面(即为了让你能够在 Google 搜索结果里秒开这个页面),如果你希望参与 V2EX 社区的讨论,你可以继续到 V2EX 上打开本讨论主题的完整版本。

https://www.v2ex.com/t/382543

V2EX 是创意工作者们的社区,是一个分享自己正在做的有趣事物、交流想法,可以遇见新朋友甚至新机会的地方。

V2EX is a community of developers, designers and creative people.

© 2021 V2EX