Skip to content
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.

Sign up for GitHub

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

Jump to bottom

#473 - Specify which class the graph is for. #480

Merged
merged 5 commits into from
Jul 26, 2020
Merged

#473 - Specify which class the graph is for. #480

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
3846ce0
#473 - Specify which class the graph is for.
vzurauskas Jun 11, 2020
89453f2
#473 - Decreased class data abstraction coupling.
vzurauskas Jun 11, 2020
06ba06d
#473 - Qulice warning and todo.
vzurauskas Jun 11, 2020
a869fac
#473 - Fixed xpaths.
vzurauskas Jun 19, 2020
847ef28
#473 - Fixed merge conflicts.
vzurauskas Jul 11, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
40 changes: 29 additions & 11 deletions src/main/java/org/jpeek/graph/XmlGraph.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,17 +24,22 @@
package org.jpeek.graph;

import com.jcabi.xml.XML;
import java.io.IOException;
import java.util.List;
import java.util.Map;
import org.cactoos.list.ListOf;
import org.cactoos.scalar.Sticky;
import org.cactoos.scalar.Unchecked;
import org.cactoos.text.FormattedText;
import org.jpeek.skeleton.Skeleton;

/**
* Graph implementation built on skeleton.
* @since 0.30.9
* @todo #473:30min Find a way to eliminate this
* ClassDataAbstractionCouplingCheck. The class probably needs to be split
* into smaller ones, perhaps extracting the maps into separate objects
* (extending MapEnvelopes), or maybe the list itself.
* @checkstyle ClassDataAbstractionCouplingCheck (500 lines)
*/
public final class XmlGraph implements Graph {

Expand All @@ -46,11 +51,15 @@ public final class XmlGraph implements Graph {
/**
* Ctor.
* @param skeleton XMl representation on whiwh to build the graph
* @param pname Package of the class this graph is for
* @param cname Class in the skeleton this graph is for
*/
public XmlGraph(final Skeleton skeleton) {
public XmlGraph(
final Skeleton skeleton, final String pname, final String cname
) {
this.nds = new Unchecked<>(
new Sticky<>(
() -> XmlGraph.build(skeleton)
() -> XmlGraph.build(skeleton, pname, cname)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@vzurauskas why is it better to make XmlGraph for one class? Does it mean we need to introduce a todo to create another class responsible for iterating on all the classes of this skeleton?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@victornoel XmlGraph implements this interface:

public interface Graph {
    List<Node> nodes();
}

It's a list of nodes for one class, because if it was for all classes in skeleton, it would have to be a list of lists of nodes, or a map to lists of nodes. As far as I can tell, the graph is useful only in a context of a single class, because I don't think there are any cross-class cohesion metrics.

Iteration could be metric-specific, I guess. E.g. Ccm iterates through classes of a transformed skeleton and has a method (not implemented currently) to update the NCC value using a graph of a specific class.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@vzurauskas ok, thank you for the explanation :)

)
);
}
Expand All @@ -63,19 +72,28 @@ public List<Node> nodes() {
/**
* Builds the graph from the skeleton.
* @param skeleton XML representation on whiwh to build the graph
* @param pname Package of the class this graph is for
* @param cname Class in the skeleton this graph is for
* @return List of nodes
* @throws IOException If fails
* @todo #445:30min This method works only for skeletons with a single
* class because it assigns all methods in the skeleton to the first class
* it finds. Instead, it should iterate through classes and then through
* methods of each class.
*/
private static List<Node> build(final Skeleton skeleton) throws IOException {
private static List<Node> build(
final Skeleton skeleton, final String pname, final String cname
) {
final Map<XML, Node> byxml = new org.cactoos.map.Sticky<>(
method -> method,
method -> new Node.Simple(
new XmlMethodSignature(
skeleton.xml().nodes("//class").get(0),
skeleton.xml()
.nodes(
new FormattedText(
"//package[@id='%s']", pname
).toString()
).get(0)
.nodes(
new FormattedText(
"//class[@id='%s']", cname
).toString()
).get(0),
method
).asString()
),
Expand All @@ -92,7 +110,7 @@ private static List<Node> build(final Skeleton skeleton) throws IOException {
final List<XML> calls = entry.getKey().nodes("ops/op[@code='call']");
final Node caller = entry.getValue();
for (final XML call : calls) {
final String name = new XmlMethodCall(call).asString();
final String name = new XmlMethodCall(call).toString();
if (byname.containsKey(name)) {
final Node callee = byname.get(name);
caller.connections().add(callee);
Expand Down
20 changes: 3 additions & 17 deletions src/main/java/org/jpeek/graph/XmlMethodArgs.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,36 +24,22 @@
package org.jpeek.graph;

import com.jcabi.xml.XML;
import java.io.IOException;
import org.cactoos.Text;
import org.cactoos.text.Joined;
import org.cactoos.text.TextEnvelope;

/**
* Serialize method arguments to a string.
*
* @since 1.0
*/
public final class XmlMethodArgs implements Text {

/**
* XML Method.
*/
private final XML method;
public final class XmlMethodArgs extends TextEnvelope {

/**
* Ctor.
*
* @param method Method as XML
*/
XmlMethodArgs(final XML method) {
this.method = method;
}

@Override
public String asString() throws IOException {
return new Joined(
":",
this.method.xpath("args/arg/@type")
).asString();
super(new Joined(":", method.xpath("args/arg/@type")));
}
}
25 changes: 8 additions & 17 deletions src/main/java/org/jpeek/graph/XmlMethodCall.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,36 +24,27 @@
package org.jpeek.graph;

import com.jcabi.xml.XML;
import java.io.IOException;
import org.cactoos.Text;
import org.cactoos.text.Joined;
import org.cactoos.text.TextEnvelope;

/**
* Serialize method call to a string.
*
* @since 1.0
*/
public final class XmlMethodCall implements Text {

/**
* XML Call operation.
*/
private final XML call;
public final class XmlMethodCall extends TextEnvelope {

/**
* Ctor.
*
* @param call Call operation as XML.
*/
XmlMethodCall(final XML call) {
this.call = call;
}

@Override
public String asString() throws IOException {
return new Joined(
"", this.call.xpath("name/text()").get(0),
".", new XmlMethodArgs(this.call).asString()
).asString();
super(
new Joined(
"", call.xpath("name/text()").get(0),
".", new XmlMethodArgs(call).toString()
)
);
}
}
6 changes: 4 additions & 2 deletions src/test/java/org/jpeek/graph/XmlGraphTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,8 @@ final class XmlGraphTest {
@Test
void buildsMethodsAsNodes() {
final List<Node> nodes = new XmlGraph(
new Skeleton(new FakeBase(XmlGraphTest.CLASS_NAME))
new Skeleton(new FakeBase(XmlGraphTest.CLASS_NAME)),
"", XmlGraphTest.CLASS_NAME
).nodes();
new Assertion<>(
"Must build nodes representing methods",
Expand Down Expand Up @@ -110,7 +111,8 @@ void buildsConnections() {
Node::name,
node -> node,
new XmlGraph(
new Skeleton(new FakeBase(XmlGraphTest.CLASS_NAME))
new Skeleton(new FakeBase(XmlGraphTest.CLASS_NAME)),
"", XmlGraphTest.CLASS_NAME
).nodes()
);
final Node one = byname.get(XmlGraphTest.METHOD_ONE);
Expand Down