[Cdk-devel] Refactoring AKA How a serious bug in AtomContainer was fixed

[Egon W. <el...@ca...>]

Hi all,

some time ago I accidentally introduced a bug in the core CDK classes. The 
limited number of Junit tests allowed me to break CDK without breaking Junit 
tests... But anyway, the bug is found and fixed (details see patch 1.28-1.29
for src/org/openscience/cdk/AtomContainer.java). This also fixed the problem 
for detection of Rings, which relied on the malfunctioning code.

Note that the bug was found by defining many new JUnit tests. There are over
130 tests right now, of which 65 test the core classes.

Note that the proper way to modify source code is applying the Refactoring
paradigma, which goes:

PHASE I : the design

1. define the new goals the classes/methodsFixed  must implement
2. identify the classes and methods that are involved

PHASE II : check current implementation

3. check that all classes and methods have associated JUnit tests
4. implementing missing JUnit tests for classes and methods
5. do the tests and makes sure all performs as required

PHASE III: apply design modifications

6. make the modifications that you think are needed to fullfil the new goals
7. make new JUnit tests the test the new/changed methods for achieving
   these goals

PHASE IV : test new implementation

8. run all JUnit tests and see if everything is working as required

After these four phases are finished, things can be uploaded to CVS.

As you might have noticed, not performing this scheme can get you in a
situation where your looking for over a month to code which might be causing 
the bug is some distantly related code, *without* ever finding the real 
problem. I've been there, done that :(

greetings,

Egon