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.

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

#396 Add ReportData class #401

Merged
merged 9 commits into from
Mar 5, 2020
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
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
56 changes: 28 additions & 28 deletions src/main/java/org/jpeek/App.java
Original file line number Diff line number Diff line change
Expand Up @@ -169,112 +169,112 @@ public void analyze() throws IOException {
if (this.params.containsKey("LCOM")) {
reports.add(
new XslReport(
chain.transform(skeleton),
"LCOM", this.params, xsl, 10.0d, -5.0d
chain.transform(skeleton), xsl,
new ReportData("LCOM", this.params, 10.0d, -5.0d)
)
);
}
if (this.params.containsKey("CAMC")) {
reports.add(
new XslReport(
chain.transform(skeleton),
"CAMC", this.params, xsl
chain.transform(skeleton), xsl,
new ReportData("CAMC", this.params)
)
);
}
if (this.params.containsKey("MMAC")) {
reports.add(
new XslReport(
chain.transform(skeleton),
"MMAC", this.params, xsl, 0.5d, 0.1d
chain.transform(skeleton), xsl,
new ReportData("MMAC", this.params, 0.5d, 0.1d)
)
);
}
if (this.params.containsKey("LCOM5")) {
reports.add(
new XslReport(
chain.transform(skeleton),
"LCOM5", this.params, xsl, 0.5d, -0.1d
chain.transform(skeleton), xsl,
new ReportData("LCOM5", this.params, 0.5d, -0.1d)
)
);
}
if (this.params.containsKey("NHD")) {
reports.add(
new XslReport(
chain.transform(skeleton),
"NHD", xsl
chain.transform(skeleton), xsl,
new ReportData("NHD")
)
);
}
if (this.params.containsKey("LCOM2")) {
reports.add(
new XslReport(
chain.transform(skeleton),
"LCOM2", this.params, xsl
chain.transform(skeleton), xsl,
new ReportData("LCOM2", this.params)
)
);
}
if (this.params.containsKey("LCOM3")) {
reports.add(
new XslReport(
chain.transform(skeleton),
"LCOM3", this.params, xsl
chain.transform(skeleton), xsl,
new ReportData("LCOM3", this.params)
)
);
}
if (this.params.containsKey("SCOM")) {
reports.add(
new XslReport(
chain.transform(skeleton),
"SCOM", this.params, xsl
chain.transform(skeleton), xsl,
new ReportData("SCOM", this.params)
)
);
}
if (this.params.containsKey("OCC")) {
reports.add(
new XslReport(
chain.transform(skeleton),
"OCC", this.params, xsl
chain.transform(skeleton), xsl,
new ReportData("OCC", this.params)
)
);
}
if (this.params.containsKey("PCC")) {
reports.add(
new XslReport(
chain.transform(skeleton),
"PCC", xsl
chain.transform(skeleton), xsl,
new ReportData("PCC")
)
);
}
if (this.params.containsKey("TCC")) {
reports.add(
new XslReport(
chain.transform(skeleton),
"TCC", xsl
chain.transform(skeleton), xsl,
new ReportData("TCC")
)
);
}
if (this.params.containsKey("LCC")) {
reports.add(
new XslReport(
chain.transform(skeleton),
"LCC", xsl
chain.transform(skeleton), xsl,
new ReportData("LCC")
)
);
}
if (this.params.containsKey("CCM")) {
reports.add(
new XslReport(
chain.transform(skeleton),
"CCM", xsl
chain.transform(skeleton), xsl,
new ReportData("CCM")
)
);
}
if (this.params.containsKey("MWE")) {
reports.add(
new XslReport(
chain.transform(skeleton),
"MWE", xsl
chain.transform(skeleton), xsl,
new ReportData("MWE")
)
);
}
Expand Down
133 changes: 133 additions & 0 deletions src/main/java/org/jpeek/ReportData.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,133 @@
/*
* The MIT License (MIT)
*
* Copyright (c) 2017-2019 Yegor Bugayenko
*
* Permission is hereby granted, free of charge, to any person obtaining a copy
* of this software and associated documentation files (the "Software"), to deal
* in the Software without restriction, including without limitation the rights
* to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
* copies of the Software, and to permit persons to whom the Software is
* furnished to do so, subject to the following conditions:
*
* The above copyright notice and this permission notice shall be included
* in all copies or substantial portions of the Software.
*
* THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
* IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
* FITNESS FOR A PARTICULAR PURPOSE AND NON-INFRINGEMENT. IN NO EVENT SHALL THE
* AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
* LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
* OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
* SOFTWARE.
*/
package org.jpeek;

import java.util.HashMap;
import java.util.Map;

/**
* Report data holder.
*
* @since 0.30.9
*/
final class ReportData {
/**
* Default mean.
*/
private static final double DEFAULT_MEAN = 0.5d;

/**
* Default sigma.
*/
private static final double DEFAULT_SIGMA = 0.1d;

/**
* The metric.
*/
private final String metr;

/**
* Mean.
*/
private final double man;

/**
* Sigma.
*/
private final double sig;

/**
* XSL params.
*/
private final Map<String, Object> args;

/**
* Ctor.
* @param name Name of the metric
*/
ReportData(final String name) {
this(
name, new HashMap<>(0), ReportData.DEFAULT_MEAN, ReportData.DEFAULT_SIGMA
);
}

/**
* Ctor.
* @param name Name of metric
* @param args Params for XSL
*/
ReportData(final String name, final Map<String, Object> args) {
this(
name, args, ReportData.DEFAULT_MEAN, ReportData.DEFAULT_SIGMA
);
}

/**
* Ctor.
* @param name Name of the metric
* @param args Params for XSL
* @param mean Mean
* @param sigma Sigma
* @checkstyle ParameterNumberCheck (10 lines)
*/
ReportData(final String name, final Map<String, Object> args, final double mean,
final double sigma) {
this.metr = name;
this.args = new HashMap<>(args);

Choose a reason for hiding this comment

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

@HDouss It is better to use unmodifiable map here

this.man = mean;
this.sig = sigma;
}

/**
* Metric name accessor.
* @return The metric
*/
public String metric() {
return this.metr;
}

/**
* Mean accessor.
* @return The mean
*/
public double mean() {
return this.man;
}

/**
* Sigma accessor.
* @return Sigma
*/
public double sigma() {
return this.sig;
}

/**
* Params accessor.
* @return Params
*/
public Map<String, Object> params() {
return new HashMap<>(this.args);

Choose a reason for hiding this comment

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

@HDouss Just returning unmodifiable map would sufficient

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@fanifieiev Ok. but I think I should remove the test for immutability or change it to expect Exceptions.

Choose a reason for hiding this comment

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

@HDouss I would personally not create a test for ReportData, it is just a holder object, just dummy object, I would better check how much it is involved in XslReportTest

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@fanifieiev it does not harm to have such tests, especially for immutability.

}
}
71 changes: 11 additions & 60 deletions src/main/java/org/jpeek/XslReport.java
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,6 @@
import com.jcabi.xml.XSLDocument;
import java.io.IOException;
import java.nio.file.Path;
import java.util.HashMap;
import java.util.Map;
import org.cactoos.collection.CollectionOf;
import org.cactoos.io.TeeInput;
Expand All @@ -52,15 +51,6 @@
* @checkstyle ClassDataAbstractionCouplingCheck (500 lines)
*/
final class XslReport implements Report {
/**
* Default mean.
*/
private static final double DEFAULT_MEAN = 0.5d;

/**
* Default sigma.
*/
private static final double DEFAULT_SIGMA = 0.1d;

/**
* Location to the schema file.
Expand All @@ -81,6 +71,11 @@ final class XslReport implements Report {
XslReport.class.getResourceAsStream("xsl/metric.xsl")
).with(new ClasspathSources());

/**
* XSL params.
*/
private final Map<String, Object> params;

/**
* The skeleton.
*/
Expand All @@ -101,70 +96,26 @@ final class XslReport implements Report {
*/
private final XSL post;

/**
* XSL params.
*/
private final Map<String, Object> params;

/**
* Ctor.
* @param xml Skeleton
* @param name Name of the metric
* @param calc Calculus
*/
XslReport(final XML xml, final String name, final Calculus calc) {
this(
xml, name, new HashMap<>(0), calc,
XslReport.DEFAULT_MEAN, XslReport.DEFAULT_SIGMA
);
}

/**
* Ctor.
* @param xml Skeleton
* @param name Name of metric
* @param args Params for XSL
* @param calc Calculus
* @checkstyle ParameterNumberCheck (5 lines)
*/
XslReport(final XML xml, final String name, final Map<String, Object> args,
final Calculus calc) {
this(
xml, name, args, calc,
XslReport.DEFAULT_MEAN, XslReport.DEFAULT_SIGMA
);
}

/**
* Ctor.
* @param xml Skeleton
* @param name Name of the metric
* @param args Params for XSL
* @param calc Calculus
* @param mean Mean
* @param sigma Sigma
* @todo #390:30min this constructor now has too many arguments. We should find a way
* to refactor the constructor or the class to have fewer parameters.
* We could start by analyzing the usage of this.params (args in this
* constructor) and get rid of it if it is not used.
* Another idea could be to have a data class contaning reporting params:
* name, args, mean, sigma.
* @param data Report data
* @checkstyle ParameterNumberCheck (10 lines)
*/
XslReport(final XML xml, final String name,
final Map<String, Object> args, final Calculus calc,
final double mean, final double sigma) {
XslReport(final XML xml, final Calculus calc, final ReportData data) {
this.skeleton = xml;
this.metric = name;
this.params = args;
this.metric = data.metric();
this.params = data.params();
this.calculus = calc;
this.post = new XSLChain(
new CollectionOf<>(
new XSLDocument(
XslReport.class.getResourceAsStream(
"xsl/metric-post-colors.xsl"
)
).with("low", mean - sigma).with("high", mean + sigma),
).with("low", data.mean() - data.sigma())
.with("high", data.mean() + data.sigma()),
new XSLDocument(
XslReport.class.getResourceAsStream(
"xsl/metric-post-range.xsl"
Expand Down
4 changes: 2 additions & 2 deletions src/test/java/org/jpeek/MetricsTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -252,8 +252,8 @@ public static Collection<Object[]> targets() {
public void testsTarget() throws Exception {
final Path output = Files.createTempDirectory("");
new XslReport(
new Skeleton(new FakeBase(this.target)).xml(),
this.metric, new XslCalculus()
new Skeleton(new FakeBase(this.target)).xml(), new XslCalculus(),
new ReportData(this.metric)
).save(output);
final String xpath;
if (Double.isNaN(this.value)) {
Expand Down
Loading