我被要求进行代码审查并报告在我们的一个新产品中添加新功能的可行性,这是我迄今为止没有亲自参与过的产品。我知道挑剔别人的代码很容易,但我会说它的状态很糟糕(同时试图尽可能客观)。我的代码审查中的一些亮点:
线程的滥用:
QueueUserWorkItem
线程通常被大量使用,线程池委托具有无意义的名称,例如PoolStart
和PoolStart2
。线程之间也缺乏适当的同步,特别是在 UI 线程以外的线程上访问 UI 对象。魔术数字和魔术字符串:一些
Const
' 和Enum
' 是在代码中定义的,但大部分代码都依赖于文字值。全局变量:许多变量被声明为全局变量,可能会或可能不会被初始化,具体取决于遵循的代码路径和发生的顺序。当代码也在线程之间跳转时,这会变得非常混乱。
编译器警告:主要解决方案文件包含 500 多个警告,我不知道总数。我收到了来自 Visual Studio 的警告,它无法再显示任何警告。
半成品类:代码被处理并添加到这里和那里,我认为这导致人们忘记了他们之前所做的事情,所以有一些看似半成品的课程和空存根。
Not Invented Here:该产品复制了其他产品使用的通用库中已经存在的功能,例如数据访问帮助程序、错误记录帮助程序和用户界面帮助程序。
关注点分离:我认为有人在阅读典型的“UI -> 业务层 -> 数据访问层” 3 层架构时将这本书颠倒了。在这个代码库中,UI 层直接访问数据库,因为业务层部分实现但由于没有充分充实而大部分被忽略,而数据访问层控制着 UI 层。大多数低级数据库和网络方法都在对主窗体的全局引用上进行操作,并直接显示、隐藏和修改窗体。在实际使用比较薄的业务层的地方,它也倾向于直接控制 UI。大多数低级代码也使用
MessageBox.Show
在发生异常时显示错误消息,并且大多数会吞下原始异常。这当然使得在尝试重构之前开始编写单元测试以验证程序的功能变得更加复杂。
我只是在这里触及表面,但我的问题很简单:花时间重构现有代码库,一次只关注一个问题,还是考虑从头开始重写整个事情是否更有意义?
编辑:澄清一下,我们确实有项目的原始要求,这就是为什么重新开始可能是一种选择。我的问题的另一种表达方式是:代码是否可以达到维护成本大于丢弃和重新开始的成本的地步?