处理代码维护的建议[关闭]

问题描述 投票:19回答:14

今年夏天,我在图像/视频实验室里一直在我的大学工作。就在最近,我的教授给了我一个由毕业生写的程序,他刚离开程序“修理”,因为它“给出了一些错误”。

该项目是用C ++编写的(似乎是学生代码中反复出现的坏标志)。我在VS08中打开了项目,并运行了项目,事实证明,“错误”是一个bad_alloc。果然,内存管理,或更准确地说,缺乏内存管理,就是问题所在。

程序员似乎喜欢在整个代码中混合mallocs,news和new [],绝对没有free,delete或delete []。更糟糕的是,所有对象似乎至少做了4-5个不相关的事情。最重要的是,这是程序员留下的评论:

//do not delete objects, it seems to cause bugs in the segmenter

从我所看到的,有一个很好的不健康的指针和引用的引用组合,并通过引用单个类函数来改变所有值,这些函数也可能是静态的。在编译时,大约有23个警告 - 比如从double转换为char时可能丢失数据,大约17个未使用的变量等等。像我这样的时候我希望C ++在大学中从不存在,并且所有实验室工作都是像python或matlab一样完成......

所以现在,教授希望我“弄乱”这个程序,这样它就可以运行数据集大约比以前大10倍。我承认,我有点害怕告诉她代码是垃圾。

StackOverflow,你们之前从未失败过,提出好的建议,所以现在我恳求,任何关于处理这类情况的建议都会受到很多赞赏。

编辑代码大约是5000 LoC

EDIT2教授决定采用最简单的方法。哪个内存越来越多。是的,因为要在这个问题上投钱......

c++ memory-management maintenance
14个回答
23
投票

首先,糟糕的代码是糟糕的代码。 Python,Java,Matlab或其他任何垃圾收集不是==好的代码。您可以轻松地花时间调试糟糕的Python代码。

话虽如此,绝对是教授的前期。告诉她代码很糟糕并向她展示一些例子。你能做的最糟糕的事情是试图隐藏问题。如果你这样做,问题不仅在于你的膝盖,而且肯定会被归咎于你。

弄清楚最佳解决方案是什么,并向她提出建议。它可能正在修复该代码,获得计算机科学系的帮助,或者用C ++或其他语言重写它。很可能她没有任何其他选择,并乐意接受你提出的任何解决方案。


1
投票

已经给出了许多关于如何处理代码本身的好建议。听起来像是一个潜在的问题可能是(可以理解)这是非程序员为非计算机类编写的。您可能会向您的教授建议,将来她会在程序员流类中(与cs教授合作)在这些情况下进行实际实施。

现在,对于每个参与者来说,这当然都有好处和不利因素。最终用户(您的教授)必须能够创建规范或至少阐明她的需求。她可能需要确信,为了做到这一点,花时间远离她宁愿做的事情是有价值的。实施团队必须能够按照该规范工作并与最终用户进行通信。如果代码实际上是一个团队而不是个人,那么实施团队将确保代码编写得相当好。

这对于“现实世界”来说都是非常好的准备,也是跨学科合作的机会,与那些了解问题领域的人一起与理解这些工具的人合作。

它为学生提供了一个“真实的”而不是一个人为的练习,并且应该让他们产生更多的热情。 (当然,取决于部门的竞争和小政治,它也可能是制造或非首发的笨蛋,但嘿,我是一个乐观主义者......)


1
投票

从教授那里隐藏问题没有任何好处。是的,抱怨你被给予了大量的垃圾可能听起来像抱怨,但隐藏事实并试图修复它将使它看起来像你最好是一个缓慢的工人,最坏的无能。我认为首先要做的是试着礼貌地告诉教授这个程序有很多问题需要清理并给出例子......教授对编程有什么了解吗?我认为这个问题没有明确说明......但是这样或那样的方式解释了这个问题并说出你认为需要解决的问题。

也就是说,我想到的下一个重要问题是,糟糕的内存管理和糟糕的类型转换是唯一的大问题,还是那些只是你从一百个严重问题中挑选出来的例子?也就是说,程序的基本结构和逻辑基本上是健全的,还是整个事情一团糟? (我猜它一团糟,但很明显我还没看过这个节目。)

这里做出的关键决定是,您是否(a)通过该计划并一次修复一堆细节错误? (b)进行认真的重组和清理?或者(c)扔掉它并从头开始重写。在这样的情况下,我经常试图选择(c),因为看起来重写比清理混乱更少工作,但很可能现有代码中嵌入了许多详细的逻辑决策。除非你有详细的,最新的需求文档 - 这是一个不太可能的,我会立即在笑声中立即解雇它 - 了解所有规则的唯一方法是阅读现有的代码并弄清楚它是什么。此时(a)或(b)变得比(c)更有效。

希望我能说“只做X”,遗憾的是你可以合理地把它放到一个帖子中,我想我们都只能说“这里有一些合理的选择......”


0
投票

替换每一个:

Type* data = (Type*)malloc(123*sizeof(Type));

std::vector<Type> data(123);

删除所有内存管理!非常简单。然后按值传递向量。


0
投票

维护的关键:

  • SCM。在Windows上,我会推荐像Mercurial(TortoiseHg client)这样的东西。创建中央受祝福的存储库,其他人可以从中获取软件以及从开发人员私有存储库提交修复的位置。
  • 问题/错误跟踪。如果你还没有,那就去吧。这对教授来说应该很容易卖。无法为Windows提供任何建议。朋友们正在使用Mantis
  • 发布管理。通常是问题跟踪系统的一部分。但是,发布本身的过程更多的是组织和政治问题,而不是技术问题。本质上,它是关于为软件的某些内部版本提供公共名称。在现实生活中,当确定实际发布的内容时,它会变得高度政治化。

5K LoC并不多。拥有SCM有助于跟踪更改并在回归的情况下返回。进行问题跟踪可帮助团队协作解决问题。在修复问题时制作次要版本(例如,您的malloc与新内存泄漏和内存泄漏)。为更大的修复程序和功能(例如,支持更大的数据集)制作主要版本。

重要的是不要急于从小变化开始。现代SCM允许大量小型提交(同上问题跟踪问题依赖树),并且应该使用它。这样可以跟踪软件行为的变化情况以及精确引入回归的变化。这通常是最无辜的变化。


-1
投票

通过添加更多RAM可以轻松解决此问题。 1-2GB应该做的伎俩。


15
投票

重构代码,一次更改一次,直到对您有意义为止。最重要的不是确切的编码策略,而是您了解它正在做什么以及为什么。

你可能最终会触及每一行代码,所以不要试图以任何特定的顺序做事 - 先把你跳出来的东西弄错,然后继续下一个,等等上。例外:如果前一个人没有使用一致的代码格式化策略,则通过自动提交器运行整个事件作为您的第一个操作。

随你写一个测试套件。


10
投票

说实话。立即告诉你的教授代码是垃圾,为什么这样。如果您在问题中列出的内容是真的,那么代码确实是废话。

你有多少时间在手边?你了解实现的算法吗? 5kLoC并不是那么多,特别是当它都是低级别的小摆动时。当您了解基础算法并且手头有足够的时间时,将其重写为2k行容易理解的代码可能比尝试修复它更好。


7
投票

5000 LoC也不错。算你自己幸运。

由于听起来内存管理是最大的问题之一,我会从那里开始。

  1. malloc替换new的每次使用。
  2. 修复编译器警告。
  3. 用向量(或更合适的数据结构)替换数组。
  4. 尽可能使用堆栈变量/引用替换原始指针,否则替换智能指针。不要担心主要的架构变化或100%的转换 - 专注于低悬的水果并清理一些主要的内存泄漏。
  5. 开始重新架构应用程序。 在应用程序中选择离散的行为/任务。 大致弄清楚它是如何工作的。 弄清楚你希望它如何工作(专注于界面)。 制定过渡计划。 执行你的计划。 重复。

6
投票

我建议遵循Michael Feathers在他的“有效使用遗留代码”一书中概述的一些步骤。即:尽快获取测试代码。

通过执行功能的单元测试可以为您提供可以自由重构而无需担心的内容。

但是,我知道说这个比实际做得容易得多。阅读本章关于Seams(您可以覆盖/挂钩的代码部分,以便您更容易地获得测试代码):http://www.informit.com/articles/article.aspx?p=359417&seqNum=3另请参阅CppUnit和CppUnitLite,这是C ++中单元测试的框架:http://c2.com/cgi/wiki?CppUnitLite

通过内存分析器运行代码。请参阅此SO链接:https://stackoverflow.com/questions/818673/memory-profiler-for-c这将帮助您找到开始添加删除/免费语句所需的位置。我也会开始尝试在其使用的API中使内存分配至少保持一致。


3
投票

在我看来,你能做的最好的事情是告诉教授代码是一团糟,除非重写一些部分,否则不会按预期工作。建议一个小代码重构,只重写需要它的部分(而不是整个程序)。

不说这个,可以给你比你已经有的更多问题:)


1
投票

作为一个止损,您可以尝试使用某种垃圾收集的内存管理实现,如dlmalloc,看看是否允许您暂时克服内存不足的障碍。

从长远来看,你将不得不理清内存管理的混乱 - 根本没有办法解决这个问题。根据您的描述,您可能还必须解决对象设计问题,但您可能首先要完成一些内存管理清理工作。

这是我接近这个的方式:

  • 找到调用malloc()的代码中的所有位置。仔细看看它们并尝试用new调用来替换它们,这样你就只需要处理一种类型的内存管理,这样就可以更容易地进行下一步了。
  • 找到new被调用的所有地方,看看你是否可以用new替换boost::shared_ptr<>的结果赋予的原始指针。这至少应该给你一些穷人的垃圾收集;当然你也应该改变接收这些指针的函数的所有函数原型来取shared_ptr<>而不是原始指针,否则你只是打扮得很乱并且没有真正改进任何东西。我实际上认为你已经改变了事情......

一旦解决了直接的内存管理问题,就可以开始重构对象并改进设计。我不会首先修复设计,你最好让软件正常工作,更好地了解它的工作原理,然后整理设计,否则你很可能用另一个来代替它。


1
投票

听起来像一团糟。重构是你需要做的最少的事情。 new和malloc的混合物是灾难的秘诀!

我想你几乎可以重写整个事情。幸运的是,5000 SLOC不是很大,不应该花太长时间。

听起来很棒!


1
投票

向教授解释你所看到的问题。如果没有解决方案,永远不会出现问题。在撰写报告和建议时请记住这一点。您可以提供多种可能性,让他们选择他们想要的。那时你将完成你的工作 - 他们做他们的时间!

© www.soinside.com 2019 - 2024. All rights reserved.