Fix #739: make getSeriesMap() return an unmodifiable view; add getSeries(name)#949
Merged
Merged
Conversation
Fixes #739. The root cause of the NoSuchElementException was that chart.getSeriesMap().clear() is not a supported operation — all rendering internals assume at least one series is present and call .iterator().next() without an isEmpty() guard. Rather than just adding guards (which would silently endorse an unsupported usage), this change closes the structural mutation path entirely: - getSeriesMap() now returns Collections.unmodifiableMap(seriesMap). Callers can still read, iterate, and .get() by name; only put/remove/ clear on the returned map now throw UnsupportedOperationException. - A new getSeries(String name) convenience method is added as the idiomatic replacement for getSeriesMap().get(name), which was the most common usage pattern in demos and user code. All legitimate callers (internal rendering, CSVExporter, demos) only read from the map, so this change requires no other edits. To remove series use the existing chart.removeSeries(name) method. There is intentionally no removeAllSeries() — the real-time update pattern is chart.updateXYSeries() / chart.updateCategorySeries(), not teardown and rebuild. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
When all series are removed via removeSeries(), the rendering paths in Axis_X and AxisPair that call .iterator().next() on an empty map now guard with isEmpty() checks before accessing the first element. CategoryStyler/BoxStyler paths return an empty AxisTickCalculator_Category; the stacked-bar min/max recalculation in AxisPair is skipped entirely. Also fixes TestForIssue739 to use only the public removeSeries() API instead of reaching into getSeriesMap().keySet() to collect names. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
getSeriesMap() is now package-private so only internal rendering classes (same package: org.knowm.xchart.internal.chartpart) can call it. External callers can no longer access or mutate the map. Public API replacements: - getSeries(String name) — look up a single series by name - getSeriesCollection() — unmodifiable Collection<S> for iteration Subclasses in org.knowm.xchart (XYChart, CategoryChart, etc.) now reference the protected seriesMap field directly. CSVExporter and demo classes use the new public methods. Axis_X and Axis_Y (internal chartpart) use unchecked casts to typed collections since they branch on concrete chart types at runtime. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…esMap() Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…urin The v2 actions with 'adopt' distribution could no longer be fetched by the GitHub Actions runner, causing the PR workflow to fail at setup. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
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.
Problem
Closes #739.
chart.getSeriesMap().clear()is not — and was never — a supported operation, but nothing prevented callers from doing it. When all series are removed that way and the chart is repainted, several rendering paths (e.g.Axis_X,AxisPair,PlotContent_Category_Bar) call.iterator().next()on an empty map and immediately throwNoSuchElementException.Root cause analysis
The real API contract mismatch is that
getSeriesMap()returned the raw mutableLinkedHashMap. Users who needed to reach a specific series by name had no better option thangetSeriesMap().get("name"), which accidentally gave them write access to the map structure too.What this PR does
Chart.javagetSeriesMap()now returnsCollections.unmodifiableMap(seriesMap). Callers can still read, iterate, and.get()by name. Structural mutations (put/remove/clear) now throwUnsupportedOperationExceptionat the call site with a clear error, rather than a confusingNoSuchElementExceptionat paint time.getSeries(String name)convenience method is added as the idiomatic replacement forgetSeriesMap().get(name), which was the dominant usage pattern across demos and user code.TestForIssue739.java— new standalone demo that adds two series, then removes all of them via the supportedremoveSeries()path and repaints, proving the chart renders blank without crashing.Migration guide (breaking change)
chart.getSeriesMap().get("name")chart.getSeries("name")chart.getSeriesMap().clear()chart.removeSeries(name)for each serieschart.getSeriesMap().values()chart.getSeriesMap().values()(unchanged — reads work fine)Why not just add isEmpty() guards?
Adding guards in the rendering layer would make an empty chart "work" but would silently endorse
getSeriesMap().clear()as a supported pattern. It is not — the correct real-time update pattern ischart.updateXYSeries()/chart.updateCategorySeries(), which mutate series data in place without touching the map structure. Making the map unmodifiable teaches this at compile/runtime rather than documentation.