in reply to Howto best review code that has not been reviewed before?
Greeting again Monks!
Thanks for all the very useful responses and links! Have a lot of ammunition for the next meeting.
Let me try to answer some of the questions that were asked in the responses. First of all it really still is safe to travel! The software to control traffic consists of several layers. The bottom layer designed under very strict procedures and safety standards. It is designed by one team, reviewed by another, and tested by yet another. It contains the logic, some hardwired, to make sure no two trains can ever make use of the same track and collide.
The software we make sits on top of this layer. Since the bottom layer is secure the guidelines for our software are less strict. If our software fails things just stop and are delayed.
We do use revision control, a 20 year old one, but we do have a nice GUI in Perl/Tk on top of it. We do do a lot of testing, but only after all code is written.
To give an idea of the problems we have in our code:
- We have a coding standard but not everybody followed the rules.
- We have functions that are over 800 lines long. They did not started out that way of course. A few years ago a function would be 60 lines long. Then the requirements changed, and again, and again; each developer added a few lines, and it turned into a 200 line function. The next developer did not have time to split it into several functions because only 8 hours were available to implement the seamingly simple change in the requirements. So the function grew and grew. Now we have 800 line monster functions that are hard to maintain and understand.
- We have software architectures that worked well a few years ago, but since they were not adapted to the evolving requirements, they do not work that well today. In response to that all kind of hacks are implemented to get around this quikcly, worsening the situation.
- We have the same functionallity implemented in different ways in different applications. (Since there was no time or budget to turn it into a library.)
A lot of this stuff, I hope, can be prevented if we get some budget for code review. Going to try to get them to adapt the approach suggested by dragonchild and etcshadow; only fix and refactor the stuff in functions that were changed,and add unittests in the process.
Management is now atleast entertaining the idea of code reviews and with this method and all the arguments supplied by the others that replied there is some light at the horizon.
Have the feeling that we are not the only shop with this kind of problem though :).
Many thanks again.