作者回复: 找到了好几个问题,很赞!
> 1. 第7行:ServerNameSpec建议增加访问修饰符:public
要是公开类,需要加public;包内部的类,可以使用缺省的修饰符。
> 2. 第11行:返回的是 UnmodifiableList 类型的List,但是在15行中使用了add方法,会抛:UnsupportedOperationException异常;
👍,是的。如果肉眼看不到,这是一个测试可以测出的错误。
> 3. 第20行:没有缩进;也没有使用大扩号来包裹代码块。
是的,需要使用大扩号和缩进,两个都要用。
> 4. 第23行:serverNames没有使用泛型,所以直接使用SNIServerName会编译不过。
是的,这是一个编译器可以发现的错误。
作者回复: (修改后的回复,我遗漏了已经没有缺省构造函数的问题,谢谢@kenes孙)
> 1. class使用public修饰比较好,一个类有一个主类
嗯,要是公开类,需要使用public修饰符。 要是包内部类,缺省的也可以,只能在包内使用。
> 2. final变量应该初始化
final变量在构造函数里初始化也可以。
> 3. Collections.unmodifiableList()生成的List无法修改
对的,这是一个绕弯的问题,你找到了👍!
> 4. if条件尽量使用括号,下面的return应该缩进
是的,括号和缩进都要有!
> 5. List没有指定泛型,遍历就不是SNIServerName 类型,应该是Object
对的,我们再声明时,就应该把泛型类型这个问题处理好。
> 6 builder.append追加两次可以改成一次,节省运算
非常好的观点!
> 以上就是我的见解,可能不正确,还请谅解
留言区就是大家畅所欲言的、开放的地方。“人人都会犯错误”那一小节,我就是想让大家彻底放下犯错误的任何包袱。想说什么就说什么,😀放开点。
作者回复: Spec一般是Specification的缩写,表示这个类表示某种实际的规范,包含特定的参数。比如RSAPublicKeySpec,表示一个RSA公开密钥是有那几个参数组成的。
作者回复: 谢谢了
作者回复: Collections.unmodifiableList规范里没写会报异常,Collections.unmodifiableList(null)就不应该抛出异常。 但是,事实的确抛出了异常。我写了个一行的代码:
List<String> list = Collections.unmodifiableList(null);
运行时的异常开起来象是:
Exception in thread "main" java.lang.NullPointerException
at java.base/java.util.Collections$UnmodifiableCollection.<init>(Collections.java:1028)
at java.base/java.util.Collections$UnmodifiableList.<init>(Collections.java:1301)
at java.base/java.util.Collections.unmodifiableList(Collections.java:1288)
Collections类规范写了,“The methods of this class all throw a NullPointerException if the collections or class objects provided to them are null. ” 但是这种表达方式并不直观,很难找到,不好用。对我来说,这是JDK/OpenJDK的一个bug。
如果你想给OpenJDK报一个bug, 请往core-libs-dev@openjdk.java.net发邮件,或者使用https://bugs.java.com/提交bug。
意外的收获!大概我们都太熟悉这个方法了,一直都没注意到这个问题。
作者回复: 赞!这真的是一个很大的问题!
作者回复: (修改后的回复,我遗漏了已经没有缺省构造函数的问题,谢谢@kenes孙)
总结的好!
> 第7行,class使用public修饰
嗯,如果是公开的类,就要有访问权限。 如果是包内部的类,使用缺省的访问权限也可以。
> 第8行, final变量应该初始化,且不能在构造函数中修改serverNames的引用
final变量,在构造函数中初始化就行。
> 空构造函数,调用add会报错
空构造函数没有声明,使用了带参的构造函数,缺省的空构造函数就没有了。
> 第11行,Collections.unmodifiableList()生成的List无法修改
这个真不是,在构造函数中,可以初始化final变量。不过,也依赖于你最后怎么改的这个代码。你要是第8行初始化了,这里就不能再次初始化了。
> 第15行,List没有指定泛型,遍历就不是SNIServerName 类型,应该是Object
嗯,我们应该在第8行声明时,就把类型这个问题解决掉。
> 第19行,if条件尽量使用括号,下面的return应该缩进
是的,缩进和括号,都要有!
> 第23行,for循环可能会空引用。循环之前需要判断serverNamers是否是null
对的,需要检查空值。
> 第24,25行,builder.append追加两次可以改成一次,节省运算
嗯,可以写成一行;写两行也没什么毛病。
你看你看,你上面的找问题,其实就是代码评审最关键的部分。人人都可以做,人人都可以找出问题。鉴于人人都会犯错误,也不要求每个问题都找对,所有的问题都找到。看代码的眼睛越多,代码的错误隐藏的机会就越小。
如果我们这样看别人的代码,看自己的代码,不用多长时间,代码质量就会有显著的提升,编码越来越轻松。加油!
作者回复: 回归测试,简单的说,就是每做一个变更,把测试都跑一遍,免得变更引起我们意想不到的麻烦。
找找有没有这方面的专栏。如果没有,你在给我留言,我们看看怎么样介绍些这方面的内容。
作者回复: 讨论区高手很多,很多留言很有参考价值,带来新的见解,也是我学习的机会。我也建议你看看讨论区不同人的不同观察角度。每一个问题,都没有标准的答案。用好讨论区,我希望讨论区的价值比专栏文章的价值还要大。当然,参与讨论的人越多钱,价值就越大,我们的收获就越多。
让我们在讨论区把练手题的疑问解决掉。为了不影响大家思考、讨论,我会稍晚几天回复练手题的问题。
作者回复: 为什么会引起空指针异常? 能多解释下吗?
作者回复:
> servernames和servername肉眼很难区分
嗯,这个点很好!
> 512定义为常量是否更加合理
这也是一个很棒的观察点,是要考虑这个问题的。
作者回复: 在我的工作场景下,这是很常见的事情。Code review最先看的不是code的细节,而是业务逻辑和设计这些更高层面的东西,然后才会去看代码的实现是不是准确。业务逻辑和设计的review,可以使用单独的文档(比如OpenJDK的CSR,JEP等),也可以使用内嵌的规范(比如Java的API规范),也可以使用代码内的注释。看不清业务逻辑的代码,要加上合理的注释;要不然,这早晚都是维护者要填的坑。
作者回复: 要分场景,参数传入,可以使用抽象类,这样的接口设计未来可以容纳更多的抽象类实现。这就是我们说的可扩展性。实例化的时候,使用实现类。
作者回复: 抱歉,我应该解释下这个背景。
SNIServerName一般用在TLS的连接中,用来指明所连接的服务器的域名。比如,我们使用https://www.example.com/,那么SNIServerName就应该设置成"www.example.com”。建立连接时,这个域名会被校验,已确保的确时连接到了“www.example.com”, 而不是一个冒牌的网站。
为了更多的灵活性,TLS的规范定义可以使用多个域名。实际上,一般情况下(比如HTTPS)一个连接仅使用一个域名。ServerNameSpec就是用来描述规范定义的,而SNIServerName就是用来描述一个域名的。
由于一般情况下,只有一个域名,所以初始化内存够一个域名用的效率就会好一些。这个问题之所以找的好,是因为,如果不去阅读相关的规范,是不会想到这一点的,这是非常专业的内容。
类似的初始化设置,如果初始化的容量和需要的容量一样大小、或者大一点点,都没有问题。但是,如果容量并不显而易见,需要计算,比如要遍历一个大的列表,我们就大致估计一个数值就好了,不要去遍历列表。如果也估计不出来,使用512算是一个常见的选择。
作者回复: 第十九行的serverNames.isEmpty(),判断的是serverNames这个集合里有没有东西。一个集合里,也可以有空值,也就是说sn可能是个空值。比如说, {null, null}就不是空集合,里面有两个空值的条目。所以,我们调用sn的方法之前,还要判断sn是不是空值。
作者回复: 你是对的,这一点我遗漏了。 谢谢!
我要回头改改给其他人的留言,免得误导了大家。
作者回复: 是的,这是一个隐蔽的较深的问题。高手!
作者回复: 是的,@Override一定不要丢了!👍
作者回复: findsbugs还在,好久不更新了。spotbugs是活跃的。