V2EX = way to explore
V2EX 是一个关于分享和探索的地方
现在注册
已注册用户请  登录
hsiunien
V2EX  ›  程序员

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

  •  
  •   hsiunien · 2017-08-13 03:17:05 +08:00 · 5906 次点击
    这是一个创建于 2420 天前的主题,其中的信息可能已经有所发展或是发生改变。

    下面这段 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<>();
            }
            
    
    44 条回复    2017-08-31 00:46:31 +08:00
    pqee
        1
    pqee  
       2017-08-13 03:32:54 +08:00 via Android
    你这个就是最健壮的,最好维护的。不过优雅就完全没有了。
    luban
        2
    luban  
       2017-08-13 04:01:25 +08:00
    可以把这个移到最外层当作默认值 studentsList = new ArrayList<>();
    wangdongjie0101
        3
    wangdongjie0101  
       2017-08-13 04:14:19 +08:00
    我觉得排版对齐就可以,写成这样肯定很乱呀。学下 python 的对齐
    340244120
        4
    340244120  
       2017-08-13 05:46:08 +08:00
    ```
    String rec = ...
    if (rec != null && students.length != 0) {
    Student[] students = convert(rec);
    studentsList = Arrays.asList(students);
    } else {
    studentsList = new ArrayList<>();
    }
    ```
    340244120
        5
    340244120  
       2017-08-13 05:46:44 +08:00
    ```java
    String rec = ...
    if (rec != null && students.length != 0) {
    Student[] students = convert(rec);
    studentsList = Arrays.asList(students);
    } else {
    studentsList = new ArrayList<>();
    }
    ```
    340244120
        6
    340244120  
       2017-08-13 05:49:55 +08:00
    为啥没有 md 效果!
    netalpha
        7
    netalpha  
       2017-08-13 05:55:43 +08:00
    需要做一些改变,对你的方法. 去掉 if 唯一的优雅方式就是使用 java8.
    我猜你得 rec 应该可以被 split, 所以:
    rec.split(",")
    .map(
    str -> {
    new Student();
    }
    )
    .collect(
    Collections.toList();
    )

    之类的. 剩下自己研究吧.大概是这么个样子.
    GavinHao
        8
    GavinHao  
       2017-08-13 06:57:08 +08:00
    @340244120 你 students 杂就直接上判断了?
    zidian
        9
    zidian  
       2017-08-13 06:58:09 +08:00
    convert
    1. 能不能返回 List 而不是 Array ?
    2. rec 是 null 的时候,返回空的 List

    于是这段代码就变成一句话了

    studentsList = convert(rec);
    zjsxwc
        10
    zjsxwc  
       2017-08-13 07:10:06 +08:00
    ```
    Student[] students ;
    studentsList = new ArrayList<>();
    String rec = ....
    boolean isStudentExists = (rec != null) && (students = convert(rec)) && (students.length > 0);
    if (isStudentExists) {
    studentsList = Arrays.asList(students);
    }
    ```
    zjsxwc
        11
    zjsxwc  
       2017-08-13 07:14:31 +08:00
    感觉写 java 是最不需要带脑子的
    twm
        12
    twm  
       2017-08-13 08:26:35 +08:00 via iPhone
    换 scala 或 kotlin
    padeoe
        13
    padeoe  
       2017-08-13 08:30:23 +08:00
    如果 convert 函数不在其他地方使用,可以让 convert 函数直接返回 list ?
    其次,既然已经把 convert 返回的 Array 转成 List,一般是为了 List 的可变长,但 Arrays.asList(students)生成的 List 不是可变的,和 ArrayList 不大一样,要小心哦
    yuanfnadi
        14
    yuanfnadi  
       2017-08-13 08:37:48 +08:00 via iPhone
    @340244120 md 要四个点
    yuanfnadi
        15
    yuanfnadi  
       2017-08-13 08:47:30 +08:00
    ````java
    studentList = (rec != null && rec.length() > 0) ? Arrays.asList(covert(rec)) : new ArrayList<>();
    ````

    这样会不会不好维护? covert 里面要做好异常判断。
    而且返回结果一个是 list 一个是 arrayList 有点奇怪。
    M3oM3oBug
        16
    M3oM3oBug  
       2017-08-13 08:57:41 +08:00 via Android
    既然 convert 方法可以解析错误的 rec 信息(也就是根据错误的 rec 返回的 students 可能为空),何不在 convert 里面添加一行错误判断呢,也就是把 rec 是否为空加到 convert 里。
    话说我忘了 Arrays.asList 是否能接收空数组对象了,若能直接一行就 OK
    cwek
        17
    cwek  
       2017-08-13 09:29:44 +08:00
    String rec = ....
    studentsList=Collections.<>emptyList();
    if (rec != null) {
    Student[] students = convert(rec);
    if (students.length != 0) {
    studentsList = Arrays.asList(students);
    }
    }
    340244120
        18
    340244120  
       2017-08-13 10:05:30 +08:00
    @GavinHao 看 Dota2TI 熬了一星期的夜,视野模糊了
    @yuanfnadi 哈哈,讲道理就是三个点,可能只有主题才支持 md 吧。
    --------------------------------------------------------
    String rec = ...
    Boolean flag ;
    Student[] students;
    if (rec != null) {
    students = convert(rec);
    if (students.length != 0) {
    flag = true;
    }
    }
    if (flag){
    studentsList = Arrays.asList(students);
    } else {
    studentsList = new ArrayList<>();
    }
    hantsy
        19
    hantsy  
       2017-08-13 10:13:06 +08:00
    @netalpha 不错,终于看到有人用 Java8 了。
    myv2ex
        20
    myv2ex  
       2017-08-13 10:16:07 +08:00   ❤️ 1
    1.Arrays.asList 可以入空数组,convert 优化一下,可去除后续 if...else 判断,如果不优化,要做非 Null 判断
    2.空 List 构造可以用 Collections.EMPTY_LIST
    little_cup
        21
    little_cup  
       2017-08-13 10:18:35 +08:00 via Android
    Optional.ofNullable(…)
    .map(this::convert)
    .stream()
    .flatmap(Arrays::stream)
    .collect(Collectors.toList());

    这样?
    hantsy
        22
    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
        23
    echotpq  
       2017-08-13 10:38:41 +08:00
    楼主应该是处女座的
    msg7086
        24
    msg7086  
       2017-08-13 11:38:14 +08:00
    if (studentsList == null)
    studentsList = new ArrayList<>();
    inisun
        25
    inisun  
       2017-08-13 11:46:33 +08:00 via iPhone
    如果不是整个项目组都会用函数式编程,写 lambda 会被人唾弃吧。
    weakish
        26
    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
        27
    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
        28
    zhx1991  
       2017-08-13 18:14:44 +08:00
    尽量提早 return, 减少 if 的层数是增加可读性的关键

    在外面 new 一个 list

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

    里面的判断也是一样的

    如果 students 是空, 直接返回

    最后一步才是赋值操作

    这样最后的代码只有一层 if, 至少看起来舒服很多
    sudit
        29
    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
        30
    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
        31
    cuebyte  
       2017-08-14 06:30:38 +08:00
    就这破代码,这么多人说没优化空间?写 Java 写傻了吧
    cuebyte
        32
    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
        33
    BBCCBB  
       2017-08-14 08:42:16 +08:00   ❤️ 1
    @mtus 此 ArrayList 非彼 ArrayList, 这是 Arrays 的一个内部类,是不可变的
    BBCCBB
        34
    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
        35
    asj  
       2017-08-14 10:29:58 +08:00
    这段代码的问题主要不在本身里面,其实你觉得不爽的地方就是空值处理。
    只需要修改 convert 函数保证不返回 null 即可。
    你本来的 if length != 0 就是没必要的, Arrays.asList 接受布丁不定长度参数,传入空数组是可以的。
    所以基本上可以简化为
    studentsList = Arrays.asList(convert(rec));
    vjnjc
        36
    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
        37
    yidinghe  
       2017-08-14 14:54:40 +08:00   ❤️ 1
    ![]( )
    这应该是楼主要的了。
    padeoe
        38
    padeoe  
       2017-08-14 16:26:03 +08:00   ❤️ 2
    @mtus 😂你在哪儿看到的这个说法...你看源码的吧,那是一个内部类,不是真·ArrayList,所以是个大坑。
    bk201
        39
    bk201  
       2017-08-14 17:28:03 +08:00
    @cuebyte 讲句不好听的,你这改写还是很 low 啊,个人觉得这种情况最好的还是 java stream 处理方式
    liyu4
        40
    liyu4  
       2017-08-14 17:50:05 +08:00
    先处理 else
    wenzhoou
        41
    wenzhoou  
       2017-08-14 21:57:16 +08:00 via Android
    可不可以写个通用 method, 这样调用:this.<List<Student>>convertTo(rec, Student::convert)
    mtus
        42
    mtus  
       2017-08-16 00:08:56 +08:00
    @BBCCBB
    @padeoe
    感谢指正, 这个 Arrays 确实是一个内部类, 是不可变的...我在测试时可能脑子进水了...
    TestCode
        43
    TestCode  
       2017-08-18 17:09:18 +08:00
    String rec = ....
    studentsList = new ArrayList<>();
    if (rec != null) {
    Student[] students = convert(rec);
    if (students.length != 0) {
    studentsList.addAll(Arrays.asList(students));
    }
    }
    hsiunien
        44
    hsiunien  
    OP
       2017-08-31 00:46:31 +08:00
    Arrays.asList(students) 出来的确实是一个内部类,不可修改,Rx 大法确实简洁很多。
    关于   ·   帮助文档   ·   博客   ·   API   ·   FAQ   ·   我们的愿景   ·   实用小工具   ·   963 人在线   最高记录 6543   ·     Select Language
    创意工作者们的社区
    World is powered by solitude
    VERSION: 3.9.8.5 · 29ms · UTC 20:34 · PVG 04:34 · LAX 13:34 · JFK 16:34
    Developed with CodeLauncher
    ♥ Do have faith in what you're doing.