Refactor CSVExporter writer handling#980
Open
PencilWarrior882 wants to merge 2 commits into
Open
Conversation
Add focused CSVExporter tests that write row-oriented and column-oriented XY series data to temporary files and verify the exact CSV content using UTF-8 reads. These tests lock the current export format before refactoring the writer implementation, so the resource-handling cleanup can be shown to preserve behavior.
Replace manual CSV Writer lifecycle management with try-with-resources so each export path closes its output stream reliably without duplicated finally blocks. Use StandardCharsets.UTF_8 instead of the UTF8 string literal, centralize the platform line separator, and remove stale commented-out CSV construction code. Public CSVExporter APIs and output format remain unchanged.
PencilWarrior882
force-pushed
the
refactor/csv-exporter-resource-handling
branch
from
ed43846 to
8fc50ea
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Refactors
CSVExporterwriter handling while preserving the existing row-oriented and column-oriented CSV output formats.Problem
CSVExportermanually managedWriterlifecycle in both export paths using duplicatedtry/finallyblocks. The close path swallowedIOExceptionwith a// NOPcomment, the UTF-8 charset was passed as a string literal, and the platform line separator lookup was repeated across the class.Refactoring
This PR replaces the manual writer cleanup with try-with-resources, uses
StandardCharsets.UTF_8, centralizes the platform line separator, and removes stale commented-out CSV construction code.The public
CSVExporterAPIs are unchanged:writeCSVRows(XYChart, String)writeCSVRows(XYSeries, String)writeCSVColumns(XYChart, String)writeCSVColumns(XYSeries, String)Impact Check
I checked the existing
CSVExportercall sites:xchart-demo/.../csv/Export2Rows.javaxchart-demo/.../csv/Export2Columns.javaXChartPanel.javaCSV export pathThe refactor keeps the same public signatures, same output orientation, same platform line separator behavior, and same broad exception handling. The removed commented-out CSV construction code referenced variables that are not present in the current method and was not a usable alternate implementation.
What This PR Adds
A focused
CSVExporterTestwith temporary-file based checks for both supported export formats:The tests read the generated CSV files as UTF-8 and verify exact content, so the refactor is covered against output-format regressions.
Validation
mvn -pl xchart "-Dtest=CSVExporterTest" testmvn fmt:checkmvn clean verify --no-transfer-progress --batch-mode --settings etc/settings.xml "-Dmaven.javadoc.skip=true" "-Dfmt.skip=true"