-
-
Notifications
You must be signed in to change notification settings - Fork 82
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
Dot Exporter and Write to a file #618
Conversation
Codecov ReportPatch coverage:
Additional details and impacted files@@ Coverage Diff @@
## develop #618 +/- ##
=============================================
+ Coverage 64.33% 64.39% +0.06%
- Complexity 3290 3297 +7
=============================================
Files 311 311
Lines 14709 14741 +32
Branches 2450 2450
=============================================
+ Hits 9463 9493 +30
- Misses 4359 4361 +2
Partials 887 887
☔ View full report in Codecov by Sentry. |
sootup.callgraph/src/main/java/sootup/callgraph/GraphBasedCallGraph.java
Outdated
Show resolved
Hide resolved
sootup.callgraph/src/main/java/sootup/callgraph/GraphBasedCallGraph.java
Outdated
Show resolved
Hide resolved
sootup.callgraph/src/main/java/sootup/callgraph/GraphBasedCallGraph.java
Outdated
Show resolved
Hide resolved
.append(";\n"); | ||
} | ||
// Sort the callGraph | ||
String sortedCallGraph = sortCallGraph(dotFormatBuilder.toString()); |
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.
dont sort the string. Sort the edges in the call graph it is way easier
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.
wasting execution time to extract strings if we have already ways to get them before we call tostring on the method signature
sootup.callgraph/src/main/java/sootup/callgraph/GraphBasedCallGraph.java
Outdated
Show resolved
Hide resolved
1. Removed the file write and now the method returns the DotFormat string 2. All methodsignatures are enclosed in "" 3.Unwanted imports removed 4.Sorted based on edges and parameter comparison added
String sortedCallGraph = sortCallGraph(dotFormatBuilder.toString()); | ||
|
||
// Using only one string builder, so deleting everything and adding the sorted callgraph | ||
dotFormatBuilder.delete(0, dotFormatBuilder.capacity()); |
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 do this.
Instead just return the string concatination
"strict digraph ObjectGraph {\n" + sortedCallgraph + "}"
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.
deleting the string builder seems unnecessary complex, and it might waste execution time
* @param dotOutput | ||
* @return | ||
*/ | ||
public String sortCallGraph(String dotOutput) { |
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 you still use this method?
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.
your edges are already kind of sorted. why sort them again?
graph.edgeSet().stream() | ||
.sorted( | ||
Comparator.comparing( | ||
(Edge edge) -> graph.getEdgeSource(edge).toString(), Comparator.naturalOrder()) |
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.
dont sort the entire signature.
otherwise the order will be class name , return type, method name and then parameter.
but we dont want that the return type highly effects the order. So you have to compare the parts of the method signatures
1. sortCallGraph() method removed 2. The comparator is written in such a way because we don't have access to methodSignature at first like in toString() method.
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.
Nice work 👍
No description provided.