Skip to content

Sonarqube Fixes#929

Merged
rfecher merged 3 commits intomasterfrom
GEOWAVE-913-2
Nov 3, 2016
Merged

Sonarqube Fixes#929
rfecher merged 3 commits intomasterfrom
GEOWAVE-913-2

Conversation

@mawhitby
Copy link
Copy Markdown
Contributor

@mawhitby mawhitby commented Nov 1, 2016

No description provided.

0
};
protected static final double[] MAXES_PER_BAND = new double[] {
private static final double[] MAXES_PER_BAND = new double[] {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

what was the sonarqube complaint here?

0
};
protected static final double[] MAXES_PER_BAND = new double[] {
private static final double[] MAXES_PER_BAND = new double[] {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

same comment...

if (y < 0.0) {
yNeg = true;
}
return (diff <= 0.0001 && diff >= -0.0001 && xNeg == yNeg);
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

suggest a smaller epsilon than .0001? ...the implication is minor, on trying to define a dateline crossing 179.99995 to -179.00005 would assume the whole world rather than across the dateline, but we should just be able to bump the epsilon to something much smaller

if (y < 0.0) {
yNeg = true;
}
return (diff <= 0.0001 && diff >= -0.0001 && xNeg == yNeg);
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

same comment...also, we should limit copy and pasted code here, have a common utility method that can be used trhoughout

private double binSize() {
final double v = (maxValue - minValue) / count.length;
return (v == 0.0) ? 1.0 : v;
return (v <= COMP_VALUE) ? 1.0 : v;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

is <= reliable? (couldn't the value v error on > 0?)

}
// entry of the the same value or first entry
if ((totalCount == 0) || (minValue == num)) {
if ((totalCount == 0L) || (Math.abs(minValue / num - 1) <= COMP_VALUE)) {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

same comment...

final double scaleX = rescaleX * (width / imageWidth);
final double scaleY = rescaleY * (height / imageHeight);
if ((scaleX != 1) || (scaleY != 1)) {
if ((Math.abs(scaleX - 1) > SIMPLIFICATION_MAX_DEGREES) || (Math.abs(scaleY - 1) > SIMPLIFICATION_MAX_DEGREES)) {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

SIMPLIFICATION_MAX_DEGREE is being used for another purpose, we probably just want a constant for DOUBLE_PRECISION_EPSILON_EQUALS or something like that defined in a utility class common to everything and re-used everywhere so we don't have various constants throughout the code

PATH_ROW_FORMATTER.setMaximumIntegerDigits(3);
PATH_ROW_FORMATTER.setMinimumIntegerDigits(3);
}
private static final String[] SCENE_ATTRIBUTES = new String[] {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

same comment as AccumuloKDEReducer - we don't want copy and pasted constants in different classes, not sure what sonarqube was complaining about, but this would not be a preferable answer

}
if (y < 0.0) {
yNeg = true;
}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

smaller epsilon? re-use code from common place?

if (y < 0.0) {
yNeg = true;
}
return (diff <= 0.0001 && diff >= -0.0001 && xNeg == yNeg);
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

same

Copy link
Copy Markdown
Contributor

@rfecher rfecher left a comment

Choose a reason for hiding this comment

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

look at embedded comments

* @param y
* @return true if the double are equal, false if they are not
*/
private static boolean checkIfDoublesEqual(
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

missed a spot?

private double binSize() {
final double v = (maxValue - minValue) / count.length;
return (v == 0.0) ? 1.0 : v;
return (v <= FloatCompareUtils.COMP_EPSILON) ? 1.0 : v;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

needs to work for positive or negative rounding error

}
// entry of the the same value or first entry
if ((totalCount == 0) || (minValue == num)) {
if ((totalCount == 0L) || (Math.abs(minValue / num - 1) <= FloatCompareUtils.COMP_EPSILON)) {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

why not Math.abs(minValue - num)?

@rfecher rfecher merged commit f37b8eb into master Nov 3, 2016
@rfecher rfecher deleted the GEOWAVE-913-2 branch November 3, 2016 13:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants