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

in cn •  8 years ago  (edited)

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 激励我创作更多更好的内容。

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

近期热贴 Recent Popular Posts

Authors get paid when people like you upvote their post.
If you enjoyed what you read here, create your account today and start earning FREE STEEM!
Sort Order:  

讨厌的机器人,抢我的沙发。

  ·  8 years ago 

没事, 顶上去,你就是沙发了。

总是听说“重新造轮子”这句话,到底是什么意思?有什么典故和出处吗?

  ·  8 years ago 

就是已经有现成的了,用就是了,
重新造的意思就是 非得自己写一份功能一样的,但实际上别人的轮子往往更好。

I just tried to understand something of this post, but I feel too dumb lol

隔行如隔山

  ·  8 years ago 

嗯, 其实 很简单。

Sure sign of a long standing code base. I have been living in one code base for 10 years now and shudder at some of the older code now.

I refactor when I need to touch areas to clean up this sort of thing :)

The other mistake I see a lot in code done around the time of C# 3.5 is misunderstanding of what is lazy and what it not. Real source of subtle bugs. Worst is mutation of stuff in a lazy evaluation.

  ·  8 years ago 

mutation is bad... that is why people love Functional Programming.

yep, big F# fan myself :) Think at least half my posts are about it lol

  ·  8 years ago (edited)

please could you write some more tutorials on F#.