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

Changed the ClusteringFactory to only do something, if there are any … #1070

Merged
merged 3 commits into from
May 15, 2023
Merged

Changed the ClusteringFactory to only do something, if there are any … #1070

merged 3 commits into from
May 15, 2023

Conversation

TwoOfTwelve
Copy link
Contributor

There was an error, if all comparisons were beneath the threshold (-m in cli). Fixed by not executing clustering, if there are no comparisons left.

@TwoOfTwelve TwoOfTwelve requested a review from Kr0nox May 9, 2023 12:36
@TwoOfTwelve
Copy link
Contributor Author

#1018

@TwoOfTwelve TwoOfTwelve linked an issue May 9, 2023 that may be closed by this pull request
Jplag throws exception if there is no comparison above -m value #1018
Closed
@tsaglam tsaglam added bug Issue/PR that involves a bug minor Minor issue/feature/contribution/change labels May 10, 2023
Kr0nox
Kr0nox approved these changes May 12, 2023
View reviewed changes
Copy link
Member

@Kr0nox Kr0nox left a comment

Choose a reason for hiding this comment

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

Looks good.

One thing I would suggest for easier readability is inverting the if statement checking for the empty Collection. That way the entire code will be one step less indented

tsaglam
tsaglam reviewed May 15, 2023
View reviewed changes
Copy link
Member

@tsaglam tsaglam left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Just one issue. Also, we could add a small unit test for the empty case. Thats always a good idea when a new bug is found.

core/src/main/java/de/jplag/clustering/ClusteringFactory.java Show resolved Hide resolved
@sonarqubecloud
Copy link

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

50.0% 50.0% Coverage
0.0% 0.0% Duplication

@tsaglam tsaglam merged commit 6a8aa5a into develop May 15, 2023
@tsaglam tsaglam deleted the bug/allComparisonsBeneathThreshold branch May 15, 2023 14:09
@tsaglam tsaglam mentioned this pull request May 23, 2023
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@tsaglam tsaglam tsaglam approved these changes

@Kr0nox Kr0nox Kr0nox approved these changes

Assignees
No one assigned
Labels
bug Issue/PR that involves a bug minor Minor issue/feature/contribution/change
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Jplag throws exception if there is no comparison above -m value
3 participants
@TwoOfTwelve @tsaglam @Kr0nox