Organizing uncommented, dirty code?

Question

I would like to ask you some questions about dirty code. There are some beginners who coded on a medium project. The code is a very huge ball of mud. They are not advanced programmers. They just know how to use keyboard an a little about java. They just wrote code with 12 000 lines in their main class, though, 6 000 lines belongs to NetBeans itself.

My job is to analyze the code and suggest a good way to maintain the code. My idea is to scrap the project and start a new one with OOP methodology. Recently I collected some notes and ideas about the problem, from this site and some others.

Now, I have the followings questions:

Should we repair the code, and change it to a OOP? We are now debugging it.
The code has no comments, no documentation, no particular style of programming, and so forth. Changing it is really expensive and time consuming. What do we can do about this?
How can I teach them to follow all the rules (commenting, OOP, good code quality, etc.)?
The code is erroneous and error prone. What can we do? Testing? We almost write two or three A4 papers for correction, but it seems endless.

I should have to say that I am new with them. I think I have broken the rules about adding people too late to the project, as well. Do you think I have to leave them?

This needs to be split into two or three questions, it's far too broad at the moment. — Jon Hopkins, Dec 04 '10 at 09:24
Yes Jeff. It is a production, a management project for administrating a financial issues! — Salivan, Dec 08 '10 at 07:18
Sorry JBR, they did not hear of a thing like that. Just making a coupe of copy paste codes all over the hard disk is their techniques to perform version controlling. — Salivan, Dec 08 '10 at 07:20
Hopking, sorry for writing too long. It was my first post, and I was unaware of how-to-post policy. Next time it will be short. — Salivan, Dec 08 '10 at 07:21
@Salivan: it's not that it's too long, its just too broad. Plus you edited the question after we gave the answers, so you make some of them irrelevant, which makes it harder for people looking for existing questions/answers to find good matches. — haylem, Dec 08 '10 at 12:02
Knowing how to use a keyboard and a bit about java is not considered a medium level programmer imo. I'd call that a script kiddie. — Chris, Jun 01 '11 at 00:14
@Salivan, if it is in production, then refactor it into shape. — , Aug 19 '11 at 20:15

score 36 · Accepted Answer · edited Apr 12 '17 at 07:31

36

Step 0: Backup to SCM

Because, as hinted to by JBRWilkinson in the comments, version control is your first line of defense against (irreversible) disaster.

Do also backup software configuration details, procedures to create deliverables, etc...

Step 1: Test First

Then start by writing tests:

for what works,
and for what fails.

No matter what you decide to do, you're covered. You can now either:

start from scratch and re-write,
or fix it.

My advice would be to start the general architecture from scratch, but extract from the mess the parts that validate checkpoints and to refactor these as you see fit.

Step 2: Verify and Monitor

Set up a Continuous Integration system (to complement step 0 and step 1) AND a Continuous Inspection system (to prepare for step 4).

Step 3: Stand on the Shoulders of Giants

(as you always should...)

Step 4: Clean

That sort of goes without saying, but instead of skimming though the code yourself, you may want to simply run linters / static analyzers and other tools on the broken codebase to find errors in the design and in the implementation.

Then you might also want to run a code formatter, that will already help a bit with the housekeeping.

Step 5: Review

It's easy to introduce tiny bugs by refactoring or cleaning things up. It only takes a wrong selection and quick hit on a key, and you might delete something fairly important without realizing at first. And sometimes the effect will appear only months later. Of course, the above steps help you to avoid this (especially by implementing a strong test harness), but you never know what can and will slip through. So make sure to have your refactorings reviewed by at least one other dedicated pair of eye-balls (and preferably more than that).

Step 6: Future-Proof your Development Process

Take all of the above, and make it an inherent part of your usual development process, if it already isn't. Don't let this happen again on your watch, and work together with your team to implement safeguards in your process and enforce this (if that's even possible) in your policies. Make producing Clean Code a priority.

But really, test. A lot.

edited Apr 12 '17 at 07:31

Community

1

answered Dec 04 '10 at 12:32

haylem

28,856
10
103
119

A great suggestion - doesn't matter what damage you do if you have tests that can catch the problems. Of course, we're all assuming that they have version control already.. – JBRWilkinson Dec 04 '10 at 18:04
@JBRWilkinson: good point actually! Indeed, completely assumed they did. – haylem Dec 04 '10 at 18:08
2

Start version control first; better late than never. – JeffO Dec 04 '10 at 19:00
@Jeff O: yes, it's already what I added to the answer. – haylem Dec 04 '10 at 19:14
Rewrote to make clearer after edits. Left attributions :) – haylem Dec 04 '10 at 19:32
Haylem, thank you my friend. But just test is not enough. Let me explain how they test the packages. They test the current buggy version again, and again and again. every time, the create a list of correction for the next test - more than fifteen items every time. The programmers, are not perfects, but after all, they do not listen to you, too. – Salivan Dec 08 '10 at 07:25
My suggestions are : 1. First letting them to be off for two or three days. I should dive into the code those days. 2. After that, talking with them about the benefits of how to write maintainable and reusable codes. To show them who it does work. 3. The next step is to prevent them to add new feature to the code. Just making them to reduce the errors for this current version. 4. One or two week will be enough to add comments, refactors. 5. The other week then will be a nice week to control them to how to write their codes under the standards. Fancy, but needs to think about it more than this. – Salivan Dec 08 '10 at 07:33
Oh, I had a discussion with the manager. He was also angry with the progress of developing the project. It has taken us, them in fact, nine months. And still it have a very long way to go. The rumor was firing them - I almost agree with him, but it is does not seem a good plan. – Salivan Dec 08 '10 at 07:36
@Salivan: I wouldn't say anything about firing at the moment, especially considering I don't know your situation. Regarding testing, of course it's not enough. But done right it will be a great help. You need unit tests for the components, integration tests for each cycle, and regression test for each new bug you fix or verify. Of course you will have some bugs coming back to haunt you but then you can catch them quickly. And developers cnnot always fix everything from one version to another. – haylem Dec 08 '10 at 10:05
@Salivan: I'll edit my answer later tonight with some more hints. – haylem Dec 08 '10 at 12:03
+1 for SCM first and foremost, but a terrific answer overall also. – Alb Feb 03 '11 at 22:49
never even noticed I didn't have a "step 1" between my 0 and 2... fixed, with an extra step to complement 0, 1 and 4 :) – haylem Jun 01 '11 at 00:13

score 15 · Answer 2 · edited Dec 04 '10 at 13:45

15

Personally, I wouldn't start this project until I have a copy of Working Effectively with Legacy Code handy. Seriously, it was written for exactly this type of thing. It's full of strategies for dealing with tricky code, and goes into far more detail than I can give you here.

edited Dec 04 '10 at 13:45

Murph

7,813
1
28
41

answered Dec 04 '10 at 08:32

Jason Baker

9,625
8
44
67

1

+1 for use of extensive external reference that says it all. – haylem Dec 04 '10 at 17:26
Unfortunately, here people have no idea about reading books. Just developing a project that it works is all they need. I started to read you-mentioned book, and CODE COMPLETE 2, too. Let me say they are wonderfully written. – Salivan Dec 08 '10 at 07:38
1

@Salivan - Perhaps they just haven't had anyone convince them that such books are worth reading. If only there were a person that worked with them who was interested in reading such books... – Jason Baker Dec 11 '10 at 08:59
1

@Salivan - the key thing is to find a quick win or two. Do something that has an almost immediate payback. Like version control, so the next time someone says "how did that happen" you can look it up. Then lead them into some suggestions from your reading of your copy of WELC. Don't just throw the book at them. – Jun 01 '11 at 00:24
2

@Salivan You can read the books for them, and drip feed the content to them as advice. You might become the team guru. – MarkJ Jun 02 '11 at 12:07

score 8 · Answer 3 · answered Dec 04 '10 at 15:03

8

I have been there several times. My rule is: if the software is not trivial (more than 1 week work for the resource you have) and it does work, then keep it and proceed with incremental refactoring.

If the software doesn't really work (very high number of bugs, unclear requirements etc.) than it's better to rewrite it from scratch. The same if it's quite small.

The point in refactoring (as in the Fowler's book and Kerievsky's one http://www.industriallogic.com/xp/refactoring/) is that it keep the system working, maybe the refactoring will take double time but the risks are zero.

Rewriting from scratch could introduce many risks, from misunderstanding requirements to wrong implementation (after all most of the team will be the same).

I actually saw a complex procedure being rewritten from scratch twice and still not working as expected.

answered Dec 04 '10 at 15:03

Uberto

980
6
12

I would also suggest writing unit tests for appropriate methods, if possible. They will help clearly define what the code is *supposed* to do in the first place, which will will aid the refactoring process. – Michael K Dec 05 '10 at 00:06
2

It goes without saying... I consider TDD a requisite for any good code (aka the new one). – Uberto Dec 05 '10 at 10:19
Writing from the very start is a very good idea. But first you need to have some diagrams of the work. But what will you do if you have to analyze the code to extract the relations? Besides, the size of the project makes it impossible, or it will make us to hire some other programmers. – Salivan Dec 08 '10 at 07:43
Test Driven Development is appreciated! – Salivan Dec 08 '10 at 07:44
"But what will you do if you have to analyze the code to extract the relations?" -> if this is the case it means the project is neither tiny nor broken. Aka I'll start doing refactoring one piece at time. See also the Mikado method. http://danielbrolund.wordpress.com/2009/03/28/start-paying-your-technical-debt-the-mikado-method/ – Uberto Dec 08 '10 at 09:18
Test driven development, my users are my guinea pigs, jk. – Chris Jun 01 '11 at 00:17
"If the software doesn't really work (very high number of bugs, unclear requirements etc.) than it's better to rewrite it from scratch." - programmers don't want to admit this, but it's true. If you have a ball of mud that only does 30% of what the customer eventually wants, and what it does is only 80% right, just tear out one module at a time and rewrite it properly. Call this one the "alpha" version. – Scott Whitlock Jun 01 '11 at 00:34

score 2 · Answer 4 · answered Dec 04 '10 at 08:14

2

I would re-write it completely. Sometimes it is impossible to repair such a code. Another option is to make it working, without adding any new features. To teach the team to write good code (well designed, documented, with tests) let them to fix the code you have now. Let everybody to fix bugs/review the code of other developers, not her/his part. After some attempts they will understand that it is almost impossible to review/fix such codes.

Adding people to late projects helps very rarely. Usually it breaks deadlines. You should do everything you can to finish the project successfully, and then think about leaving.

answered Dec 04 '10 at 08:14

duros

2,464
1
21
25

How much would it cost to re-write it completely versus iteratively improving it? Which approach would give results quickest? – JBRWilkinson Dec 04 '10 at 18:03
@JBRWilkinson It depends. The iterative approach is good, when you have working code. – duros Dec 04 '10 at 18:09
1

@duros, for broken code, yes. This code runs in production. – Aug 19 '11 at 20:19

score 2 · Answer 5 · edited Jun 16 '20 at 10:01

2

My advice will be not to scrap the entire code entirely. This is the day-to-day life issue, every development team face. Attack one part of the code at a time. Fix it, clean it, document it. And then move to the other part. The main thing is always keep some shipable code at the hand. Rewriting the entire code from the scratch will take the amount of the time that has been spent till now and there won't be any guarantee that it will be nicer than the current.
But then also the people should avoid writing the code in this manner. Spend some more time in code reviews. Adapt to some uniform coding style. Discuss the design first and then write the code. Such simple things will make big changes.

Nice Blog telling why Netscape loose

edited Jun 16 '20 at 10:01

Community

1

answered Dec 04 '10 at 09:01

Manoj R

4,076
22
30

2

Starting a new project, while updating/debugging the old version in the mean time (and you cannot avoid this so don't dream about it) is trying to shoot at multiple moving targets. – JeffO Dec 04 '10 at 19:03
"Attack one part of the code at a time" did not work, Manjo. with a deep sorrow, the code contains lots of errors. It always turns to something else. We should attack and destroy one part of the code and construct it then. I suggested this thought to the manager once, but the coders need to be off of writing any new codes. – Salivan Dec 08 '10 at 07:48
@Salivan ... _need to be off of writing any new codes_ . I'm sure management is saying this. But the first thing to do when you are in a hole is to stop digging (don't continue to make the same mistakes). To allow more to be created under these conditions is to continue digging the hole. The difficult part is how to get management and the coders to understand the issues. – SeraM Dec 19 '16 at 17:44

score 1 · Answer 6 · answered Dec 04 '10 at 17:13

1

If it works, refactor it. There are tools to help you do that. If it doesn't work, use the magical code improvement command, i.e. deltree on Windows resp. rm -rf on Linux.

answered Dec 04 '10 at 17:13

user281377

28,352
5
75
130

2

Suggesting to "completely erase all the code" is particularly unhelpful - do you have a more constructive answer? – JBRWilkinson Dec 04 '10 at 18:01
LOL. I am completely agree with you, ammoQ! – Salivan Dec 08 '10 at 07:50
JBRWilkinson: Doing a fresh restart is most likely a better approach than trying to make the mess work and clean. A company I've worked for tried that, and year after year, they had wasted lots of resources and got absolutely nowhere. – user281377 Dec 08 '10 at 09:20
@ammoQ, you need the old code to see what it _actually_ did, when you get it wrong. – Aug 19 '11 at 20:20
1

Thorbjorn: We are talking about code that *doesn't work*, right? Analyzing uncommented, dirty code that doesn't do the right thing tells me more about the mental condition of its creators than anything else. – user281377 Aug 22 '11 at 07:44

Martin Wickman · Answer 7 · 2010-12-04T18:48:54.363

Should we repair the code, and change it to a OOP? We are now debugging it. [... contains errors, no documention ...]

I've been there, you have my sympathies. I even wrote an article about this which might help you get some perspective. But in short:

If the code contains lots of duplication, you should rewrite. If there is no discernible structure (no clear interfaces, spaghetti), refactoring will fail and you should probably rewrite.

How can I teach them to follow all the rules?

Begin with explaining why they might want to do that by showing them what they can gain from it personally. When they agree with this and are willing to learn, start teaching them using shuhari.

Thanks Martin. _"Begin with explaining why they might want to do that by showing them what they can gain from it personally. When they agree with this and are willing to learn, start teaching them using shuhari."_ — Salivan, Dec 08 '10 at 07:52

score 0 · Answer 8 · answered Dec 04 '10 at 09:59

My suggestion is a combination of @duros's & @Manoj R's answers.

Start from scratch, keeping in mind to create good code/OOP/commented/etc this time, refer/copy&paste from your old code. As you meet the bad parts of your old code, rewrite/refactor them.

If your developers are not well trained, I think its good to send them for courses. Its important for regular retraining in the fast changing IT industry