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

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

  •  
  •   hsiunien · 69 天前 · 3207 次点击
    这是一个创建于 69 天前的主题,其中的信息可能已经有所发展或是发生改变。

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

    之类的. 剩下自己研究吧.大概是这么个样子.
        8
    GavinHao   69 天前
    @340244120 你 students 杂就直接上判断了?
        9
    zidian   69 天前
    convert
    1. 能不能返回 List 而不是 Array ?
    2. rec 是 null 的时候,返回空的 List

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

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

    这样会不会不好维护? covert 里面要做好异常判断。
    而且返回结果一个是 list 一个是 arrayList 有点奇怪。
        16
    M3oM3oBug   69 天前 via Android
    既然 convert 方法可以解析错误的 rec 信息(也就是根据错误的 rec 返回的 students 可能为空),何不在 convert 里面添加一行错误判断呢,也就是把 rec 是否为空加到 convert 里。
    话说我忘了 Arrays.asList 是否能接收空数组对象了,若能直接一行就 OK
        17
    cwek   69 天前
    String rec = ....
    studentsList=Collections.<>emptyList();
    if (rec != null) {
    Student[] students = convert(rec);
    if (students.length != 0) {
    studentsList = Arrays.asList(students);
    }
    }
        18
    340244120   69 天前
    @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<>();
    }
        19
    hantsy   69 天前
    @netalpha 不错,终于看到有人用 Java8 了。
        20
    myv2ex   69 天前   ♥ 1
    1.Arrays.asList 可以入空数组,convert 优化一下,可去除后续 if...else 判断,如果不优化,要做非 Null 判断
    2.空 List 构造可以用 Collections.EMPTY_LIST
        21
    little_cup   69 天前 via Android
    Optional.ofNullable(…)
    .map(this::convert)
    .stream()
    .flatmap(Arrays::stream)
    .collect(Collectors.toList());

    这样?
        22
    hantsy   69 天前
    就一数据转化而已。

    贴一段数据库初始化的代码,主要是用 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...")
    );
    ```
        23
    echotpq   69 天前
    楼主应该是处女座的
        24
    msg7086   69 天前
    if (studentsList == null)
    studentsList = new ArrayList<>();
        25
    inisun   69 天前 via iPhone
    如果不是整个项目组都会用函数式编程,写 lambda 会被人唾弃吧。
        26
    weakish   69 天前
    `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 就报错,要啰嗦一点,效果也是一样的。
        27
    mtus   69 天前
    @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);
    }
    ```
        28
    zhx1991   69 天前
    尽量提早 return, 减少 if 的层数是增加可读性的关键

    在外面 new 一个 list

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

    里面的判断也是一样的

    如果 students 是空, 直接返回

    最后一步才是赋值操作

    这样最后的代码只有一层 if, 至少看起来舒服很多
        29
    sudit   68 天前
    studentList = (rec == null)?new ArrayList<>() :
    Stream.of(rec.split(separator))
    .map(Student::convert)
    .collect(Collectors.toList());
    每个人都有对优雅的理解吧
        30
    kyuuseiryuu   68 天前
    ```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);
    ```
        31
    cuebyte   68 天前
    就这破代码,这么多人说没优化空间?写 Java 写傻了吧
        32
    cuebyte   68 天前
    首先这 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);
    }
        33
    BBCCBB   68 天前   ♥ 1
    @mtus 此 ArrayList 非彼 ArrayList, 这是 Arrays 的一个内部类,是不可变的
        34
    BBCCBB   68 天前
    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();
        35
    asj   68 天前
    这段代码的问题主要不在本身里面,其实你觉得不爽的地方就是空值处理。
    只需要修改 convert 函数保证不返回 null 即可。
    你本来的 if length != 0 就是没必要的, Arrays.asList 接受布丁不定长度参数,传入空数组是可以的。
    所以基本上可以简化为
    studentsList = Arrays.asList(convert(rec));
        36
    vjnjc   68 天前
    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{
    //业务逻辑
    }

    因为一般异常处理比较简单,所以写前面。
        37
    yidinghe   68 天前   ♥ 1
    ![]( )
    这应该是楼主要的了。
        38
    padeoe   68 天前   ♥ 2
    @mtus 😂你在哪儿看到的这个说法...你看源码的吧,那是一个内部类,不是真·ArrayList,所以是个大坑。
        39
    bk201   68 天前
    @cuebyte 讲句不好听的,你这改写还是很 low 啊,个人觉得这种情况最好的还是 java stream 处理方式
        40
    liyu4   68 天前
    先处理 else
        41
    wenzhoou   68 天前 via Android
    可不可以写个通用 method, 这样调用:this.<List<Student>>convertTo(rec, Student::convert)
        42
    mtus   66 天前
    @BBCCBB
    @padeoe
    感谢指正, 这个 Arrays 确实是一个内部类, 是不可变的...我在测试时可能脑子进水了...
        43
    TestCode   64 天前
    String rec = ....
    studentsList = new ArrayList<>();
    if (rec != null) {
    Student[] students = convert(rec);
    if (students.length != 0) {
    studentsList.addAll(Arrays.asList(students));
    }
    }
        44
    hsiunien   51 天前
    Arrays.asList(students) 出来的确实是一个内部类,不可修改,Rx 大法确实简洁很多。
    DigitalOcean
    关于   ·   FAQ   ·   API   ·   我们的愿景   ·   广告投放   ·   鸣谢   ·   1634 人在线   最高记录 3541   ·  
    创意工作者们的社区
    World is powered by solitude
    VERSION: 3.9.8.0 · 74ms · UTC 14:07 · PVG 22:07 · LAX 07:07 · JFK 10:07
    ♥ Do have faith in what you're doing.
    沪ICP备16043287号-1