-
Notifications
You must be signed in to change notification settings - Fork 221
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
ClassHierarchy toJson Function #1392
base: master
Are you sure you want to change the base?
Conversation
…t for AnalysisScope
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for this! I've added some initial comments. I think this is on the right track but there is some significant cleanup possible.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
couple more quick comments
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looking better, but still needs a bit of work
@aakgna please fix the CI builds and then I can look again |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
couple more comments
Iterator<Node> children = root.getChildren(); | ||
Set<String> subclassNames = new HashSet<>(); | ||
while (children.hasNext()) { | ||
Node temp = children.next(); | ||
subclassNames.add(nodeToString(temp)); | ||
helperToJson(temp, classNameToSubclassNames); | ||
} | ||
// Removes unnecesarry parts from name of the class | ||
String key = nodeToString(root); | ||
// inserting the root class to its subclasses into the main hashmap | ||
classNameToSubclassNames.put(key, subclassNames); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why do we need this code? Can't you just call helperToJSON(root, classNameToSubclassNames)
and that will do what you want?
Object unused = cha.toJson(); | ||
// assertEquals("", json); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Don't we need to write some actual assertions about what we expect to be in the final JSON string?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looking better! I did notice a gap in the logic, see below
Set<String> subclassNames = new HashSet<>(); | ||
|
||
// Process all children of the current node | ||
if (n.children.size() > 0) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
By printing out the JSON in the test and looking it over, I realized that it only handles subclassing, but not interface implementations. E.g., for the interface java.util.Map
we see "java.util.Map":[]
in the output, missing all the classes that implement that interface. We should update this logic so that for interfaces, we treat all implementing classes and sub-interfaces as subtypes in the outputted JSON. In other words, the JSON should map each class or interface to all of its immediate subtypes, i.e., direct subclasses, direct sub-interfaces, or classes directly implementing an interface.
Creates a JSON variable containing a map from the Root Class to its Subclasses.