Java: best practices for turning someone else's horror code into a pure API ...?

Question

I have a project (related to graph algorithms). It is written by someone else.

The code is terrible:

• public fields, without getters / setters
• huge methods, all public
• some classes have more than 20 fields
• some classes have more than 5 constructors (which are also huge)
• some of these constructors just leave many null fields
  (therefore, I cannot make some fields final, because then every second constructor signals errors)
• methods and classes rely on each other in both directions

I need to rewrite this in a clean and understandable API.

The problem is that I myself do not understand anything in this code.

Please give me advice on analyzing and understanding such code.

I thought maybe there are tools that do static code analysis and give me call graphs, etc.

Answer 1

Oh my god :-) I envy you and not at the same time ... let them take one at a time. You can solve some of these things yourself before you install the code analysis tool on it. Thus, you will get a better understanding and be able to act much further than with a simple tool.

• public fields, without getters / setters
  • make everything private. Your rule should restrict access as much as possible.
• huge methods, all public
  • Divide and make private, where it makes sense to make it
• some classes have more than 20 fields
  • ugh..The Builder in Effective Java 2nd Ed is the primary candidate for this.
• some classes have more than 5 constructors (which are also huge)
  • Telescopic constructors like the ones above seem to help
• some of these constructors just left a lot of null fields
  • yep are telescopic constructors :)
• methods and classes rely on each other in both directions
  • It will be the least pleasure. Try removing inheritance if you have completely misunderstood what is required and use composition instead of interfaces where applicable.

Good luck we are here to help

Answer 2

WOW!

I would recommend: write unittests and then start refactoring

 * public fields, no getters/setters 

start by making them confidential and "feel" the resistance of compiler errors as a metric.

 * huge methods, all public 

understand their semantics, try to introduce interfaces

 * some classes have over 20 fields 

very common in complex applications, nothing bothers

 * some classes have over 5 constructors (which are also huge) 

replace them with buider / creator pattern

 * some of those constructors just left many fields null 

see above answer

 * methods and classes rely on each other in both directions 

decide whether to rewrite everything (to be honest, I came across a cover where only 10% of the code was required)

Answer 3

Well, the eclipse cleaning master will scratch off a noticeable percentage of deposition.

Then you could point Sonar to him and fix everything he complains about if you live long enough.

Answer 4

For static analysis and call graphs (without graphs, but graph structures) you can use the Finder Finder .

Answer 5

Use an IDE that knows something about refactoring, such as IntelliJ. You will not have situations when you move one method and five other classes complain, because IntelliJ is smart enough to make all the necessary changes.

Unit tests are required. Someone refactoring without unit tests is like a high-wire performer without a protective mesh. Take it before you start a long, heavy climb.

Answer 6

The answer may be: patience and coffee.

Answer 7

So I would do this:

• Start using code, for example. from the main method, as if it were used by other classes - the same arguments, the same access orders. Do this inside the debugger, as you see every step that this class takes.
• Start writing unit tests for this functionality. Once you have reached reasonable coverage, you will begin to notice that this class probably has too many responsibilities.

while ( responsibilities != 1 ) {

• Retrieve an interface that expresses one responsibility of this class.
• Force all callers to use this interface instead of a specific type;
• Extract the implementation into a separate class;
• Pass the new class to all callers using the new interface.

}

Answer 8

Non-talking tools such as Sonar, FindBugs, etc., which some have already mentioned, do not help, but there are no tricks. Start with what you understand, create a unit test for it, and as soon as it starts the green start of refactoring in parts. Remember to fool addictions as you progress.

Answer 9

Sometimes it's easier to rewrite something from scratch. Is this "awful code" working as intended or full of errors? Is this documented?

In my current project, removing my predecessor almost entirely and rewriting it from scratch was the most effective approach. Of course, this was an extreme case of code obfuscation, a complete lack of meaningful comments and complete incompetence, so your mileage may vary.

Answer 10

Although some legacy code may be barely understandable, it can still be reorganized and improved to legibility in stages. Have you seen Joshua Kerievsky's book Refactoring for Templates ? - It's good.