ennerf commented 1 year ago

Added an experimental BenchPlugin that displays a live benchmark of the current chart:

Clicking again or closing the window disables the measurements again

codecov[bot] commented 1 year ago

Codecov Report

Patch coverage has no change and project coverage change: -0.07% :warning:

Comparison is base (16a1602) 48.00% compared to head (6dce285) 47.93%.

Additional details and impacted files

```diff @@ Coverage Diff @@ ## main #618 +/- ## ============================================ - Coverage 48.00% 47.93% -0.07% - Complexity 6180 6182 +2 ============================================ Files 394 395 +1 Lines 38176 38235 +59 Branches 6085 6087 +2 ============================================ + Hits 18325 18327 +2 - Misses 18703 18762 +59 + Partials 1148 1146 -2 ``` | [Files Changed](https://app.codecov.io/gh/fair-acc/chart-fx/pull/618?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=fair-acc) | Coverage Δ | | |---|---|---| | [...io/fair\_acc/chartfx/bench/LiveDisplayRecorder.java](https://app.codecov.io/gh/fair-acc/chart-fx/pull/618?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=fair-acc#diff-Y2hhcnRmeC1jaGFydC9zcmMvbWFpbi9qYXZhL2lvL2ZhaXJfYWNjL2NoYXJ0ZngvYmVuY2gvTGl2ZURpc3BsYXlSZWNvcmRlci5qYXZh) | `0.00% <0.00%> (ø)` | | | [.../java/io/fair\_acc/chartfx/plugins/BenchPlugin.java](https://app.codecov.io/gh/fair-acc/chart-fx/pull/618?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=fair-acc#diff-Y2hhcnRmeC1jaGFydC9zcmMvbWFpbi9qYXZhL2lvL2ZhaXJfYWNjL2NoYXJ0ZngvcGx1Z2lucy9CZW5jaFBsdWdpbi5qYXZh) | `0.00% <0.00%> (ø)` | | | [...c/main/java/io/fair\_acc/chartfx/utils/FXUtils.java](https://app.codecov.io/gh/fair-acc/chart-fx/pull/618?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=fair-acc#diff-Y2hhcnRmeC1jaGFydC9zcmMvbWFpbi9qYXZhL2lvL2ZhaXJfYWNjL2NoYXJ0ZngvdXRpbHMvRlhVdGlscy5qYXZh) | `82.55% <0.00%> (-1.13%)` | :arrow_down: | ... and [1 file with indirect coverage changes](https://app.codecov.io/gh/fair-acc/chart-fx/pull/618/indirect-changes?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=fair-acc)

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

pr-explainer-bot[bot] commented 1 year ago

Pull Request Review

Hey there! 👋 Here's a summary of the previous tasks and their results. Let's dive in!

Changes

Line 51: The setTitle method in the createChart method is setting the title of the timeChart to just title. It would be better to include the prefix 'Benchmark: ' as it was before.
Line 118: The BenchPlugin class is missing a package declaration. It should be added at the top of the file.

Suggestions

Line 51: Instead of hardcoding the title in the setTitle method, it would be better to pass it as a parameter to the method.
Line 118: The filterFunc variable in the BenchPlugin class could be renamed to something more descriptive, like measurementFilter.
Line 118: The filterFunc variable in the BenchPlugin class could be made final since it is not reassigned.
Line 118: The filterFunc variable in the BenchPlugin class could be extracted to a separate method for better readability.
Line 118: The filterFunc variable in the BenchPlugin class could be documented with a Javadoc comment explaining its purpose and usage.
Line 180: The getChartPanel method in the ChartPerformanceGraph class could be renamed to something more descriptive, like createChartPanel.
Line 180: The getChartPanel method in the ChartPerformanceGraph class could be made static since it does not rely on any instance variables.
Line 180: The getChartPanel method in the ChartPerformanceGraph class could be extracted to a separate class for better separation of concerns.
Line 180: The getChartPanel method in the ChartPerformanceGraph class could be documented with a Javadoc comment explaining its purpose and usage.
Line 260: The managedVisibilityProperty method in the FXUtils class could be renamed to something more descriptive, like bindManagedToVisible.
Line 260: The managedVisibilityProperty method in the FXUtils class could be documented with a Javadoc comment explaining its purpose and usage.

Bugs

There are no potential bugs found in the code.

Improvements

In ErrorDataSetRendererSample.java at line 21, consider extracting the creation of BenchPlugin into a separate method for better code organization and readability.
In ErrorDataSetRendererStylingSample.java at line 454, consider extracting the creation of BenchPlugin into a separate method for better code organization and readability.
In ErrorDataSetRendererSample.java at line 21, the creation of BenchPlugin can be refactored for better readability:
```
private void addBenchPlugin(XYChart chart) {
chart.getPlugins().add(new BenchPlugin());
}
```
In ErrorDataSetRendererStylingSample.java at line 454, the creation of BenchPlugin can be refactored for better readability:
```
private void addBenchPlugin(XYChart chart) {
chart.getPlugins().add(new BenchPlugin());
}
```

Rating

Overall rating: 7.5/10

Readability: The code is generally readable, but there are some areas that could be improved with better variable naming and code organization.
Performance: The code seems to be performing well and there are no obvious performance issues.
Security: The code does not contain any security vulnerabilities.

That's it for the summary! If you have any questions or need further clarification, feel free to reach out. Happy coding! 😄🚀