This is a code change that I observe today. It has been in the codebase for a long time, and I find it interesting to talk about it.

今天在看代码修改记录的时候发现有这么一处改动，虽然这个改动已经很久了，但是我觉得有必要拿出来大家讨论一下：

The LINQ is out since .NET 3.5, and all above code is just doing one thing: select the specific types e.g. either LayoutAnt or LayoutDevice from a member list layoutList . And one line of LINQ does this exactly: OfType<>

2007年 .NET 3.5 之后推出LINQ，其实整个函数只是在做一件事，就是返回类成员 layoutList 中是 LayoutDevice （后面改成LayoutAnt ）的列表。但实际上这通过 C#的LINQ只需要用 OfType<LayOutDevice> 或者 OfType<LayOutAnt> 即可（暂且不说改动包括重构类型）

The left version is OK, but it is just old-school considering the developer who did not get familiar with LINQ. But the right version is worse. It has added a ref List which will be cleared. This is not unit-testing friendly at all.

左边的版本实际上是OK的，这就是学校的标教科书版本，无可厚非，但右边的这个版本就大有问题，因为参数含有引用 ref, 也就是说每次都把外面传进的变量给清空了，这种函数拿来单元测试并不友好。

The private methods are not unit-test friendly. And one big issue for both methods is: the member variable layoutList is referenced. If you really have to re-invent the wheel, at least make it a public, static method which takes a readonly layoutList and returns a new copy of List. In this case, the entire function is immutable, and unit-test friendly.

如果一定要重新造轮子，两个版本都有小问题，一个是 private 方法不好单元测试，另一个是都使用了成员变量 layoutList, 最好是改成 public static 公有静态方法，传入 layoutList, 然后像第一种方式返回新的List。这样的话，这个公有静态方法就是不会更改 (immutable), 较容易被单元测试。

Originally published at https://steemit.com Thank you for reading my post, feel free to Follow, Upvote, Reply, ReSteem (repost) @justyy which motivates me to create more quality posts.

原创 https://Steemit.com 首发。感谢阅读，如有可能，欢迎Follow, Upvote, Reply, ReSteem (repost) @justyy 激励我创作更多更好的内容。

// 稍后同步到我的中文博客和英文算法博客。

[Code Review] - Reinventing the Wheel 代码审核之重新造轮子

近期热贴 Recent Popular Posts

[Code Review] - Reinventing the Wheel 代码审核之 重新造轮子

近期热贴 Recent Popular Posts

[Code Review] - Reinventing the Wheel 代码审核之重新造轮子