Home » Language IDEs » Java Development Tools (JDT) » God classes in JDT
| | | | | | | | | | | | | | | | | | | | |
Re: God classes in JDT [message #845089 is a reply to message #842907] |
Sat, 14 April 2012 18:37 |
Zhongpeng Lin Messages: 4 Registered: April 2012 |
Junior Member |
|
|
Thanks Stephan for the reply. That gave much insight into these two classes.
Looking into the method lists for both classes as an outsider, I found many methods in each class share common prefixes, which are all verbs; they are only different in the object they process. Example in org.eclipse.jdt.internal.codeassist.CompletionEngine:
private int computeRelevanceForAnnotation()
private int computeRelevanceForAnnotationTarget(TypeBinding typeBinding)
private int computeRelevanceForClass()
private int computeRelevanceForEnum()
private int computeRelevanceForEnumConstant(TypeBinding proposalType)
Also, the consumeXXX methods in org.eclipse.jdt.internal.compiler.parser.Parser is another example. At the same time, switch statement is used to call different methods based on different object type (e.g. org.eclipse.jdt.internal.compiler.parser.Parser.consumeRule). These situations seem to show opportunities to take advantage of polymorphism, as suggested by Martin Fowler in his book Refactoring: Improving the Design of Existing Code. By using polymorphism, different processing logic will be encapsulated in different objects/classes, and the size of these two classes will be reduced. This should work at least "in theory", and I believe developers in JDT are aware of this, but I am interested in what difficulty prevented them from doing such a refactoring.
Any more insights? Thanks again!
Stephan Herrmann wrote on Thu, 12 April 2012 14:20Zhongpeng Lin wrote on Thu, 12 April 2012 20:10I found there are two classes in JDT with more than 10 KLOC:
codeassist/org/eclipse/jdt/internal/codeassist/CompletionEngine.java
This class indeed causes some pain every now and then. For regular JDT/Core development it is manageable, but I also maintain a fork of the JDT/Core (for Object Teams) and there merging changes into this class did cause significant grief in the past.
I shyly suggested refactoring that class, which stalled probably because also an appropriate refactoring would be quite a challenge at this point. It would have been easier to split this way earlier in the development, I guess.
Quote:
compiler/org/eclipse/jdt/internal/compiler/parser/Parser.java
For this class the huge size is quite natural: a long method (consumeRule) is generated by the parser generator (jikespg) and all methods called from there must be found in the same class. Size isn't causing much harm here, since methods are quite uniform, so complexity isn't too high, actually.
HTH,
Stephan
[Updated on: Sat, 14 April 2012 18:39] Report message to a moderator
|
|
|
Re: God classes in JDT [message #845108 is a reply to message #845089] |
Sat, 14 April 2012 19:06 |
Stephan Herrmann Messages: 1853 Registered: July 2009 |
Senior Member |
|
|
Zhongpeng Lin wrote on Sat, 14 April 2012 20:37Thanks Stephan for the reply.
... and sorry for the many dupes. I'm not sure if it was my browser or the forum software that ran berserk. Hope it won't happen again.
Quote:
Looking into the method lists for both classes as an outsider, I found many methods in each class share common prefixes, which are all verbs; they are only different in the object they process. Example in org.eclipse.jdt.internal.codeassist.CompletionEngine:
private int computeRelevanceForAnnotation()
private int computeRelevanceForAnnotationTarget(TypeBinding typeBinding)
private int computeRelevanceForClass()
private int computeRelevanceForEnum()
private int computeRelevanceForEnumConstant(TypeBinding proposalType)
Also, the consumeXXX methods in org.eclipse.jdt.internal.compiler.parser.Parser is another example. At the same time, switch statement is used to call different methods based on different object type (e.g. org.eclipse.jdt.internal.compiler.parser.Parser.consumeRule).
As mentioned the Parser case is constrained by the interaction with the parser generator, so I'll focus on CompletionEngine in this reply.
Quote:These situations seem to show opportunities to take advantage of polymorphism, as suggested by Martin Fowler in his book Refactoring: Improving the Design of Existing Code. By using polymorphism, different processing logic will be encapsulated in different objects/classes, and the size of these two classes will be reduced. This should work at least "in theory",
There is no difference between theory and practice, at least in theory
Quote: and I believe developers in JDT are aware of this, but I am interested in what difficulty prevented them from doing such a refactoring.
Any more insights? Thanks again!
When I last looked at possibilities to split CompletionEngine into several classes there was one particular question that made me scratch my head: how should we handle fields?
- easiest way would be to leave all state in the current class and let extracted classes access those fields externally (by way of one back reference to the CompletionEngine instance).
- could fields be meaningfully grouped so that also some fields could be extracted?
Ergo: when addressing the God Class smell we should also have a good answer to the God Object smell (i.e., not only the class but also its instances are large).
Next we would need a call graph of all methods to find an meaningful ordering to avoid cyclical call structures where extracted objects would call back into the "master" object etc. because a control flow that busily jumps back and forth between objects may actually be much harder to understand than the current implementation.
Both issues are of course addressable, but I hope this illustrates why I felt this would require a considerable chunk of time to find a separation that not only works but actually improves understandability and thus maintainability.
Additionally: any such refactoring would essentially render the version history of that file useless: for any particular line of code after the refactoring it will be pretty difficult to see "through" the refactoring to find when that line was introduced / last changed.
Ergo: practice shows that the difference between theory and practice is much larger than you ever dreamt of
best,
Stephan
|
|
| |
Re: God classes in JDT [message #873366 is a reply to message #842907] |
Fri, 18 May 2012 04:19 |
Zhongpeng Lin Messages: 4 Registered: April 2012 |
Junior Member |
|
|
Stephan Herrmann wrote on Thu, 12 April 2012 14:20Zhongpeng Lin wrote on Thu, 12 April 2012 20:10I found there are two classes in JDT with more than 10 KLOC:
codeassist/org/eclipse/jdt/internal/codeassist/CompletionEngine.java
This class indeed causes some pain every now and then. For regular JDT/Core development it is manageable, but I also maintain a fork of the JDT/Core (for Object Teams) and there merging changes into this class did cause significant grief in the past.
Hello Stephan,
Could you give more details about what pain CompletionEngine has caused to the maintenance of the project?
Thanks!
[Updated on: Fri, 18 May 2012 04:20] Report message to a moderator
|
|
|
Goto Forum:
Current Time: Thu Sep 19 19:07:43 GMT 2024
Powered by FUDForum. Page generated in 0.05217 seconds
|