Skip to content

Fix:Harden ULongArray.reorder() validation and improve subset edge-case safety#438

Open
vishwas-droid wants to merge 1 commit intoERDDAP:mainfrom
vishwas-droid:fix/ulongarray-reorder-validation
Open

Fix:Harden ULongArray.reorder() validation and improve subset edge-case safety#438
vishwas-droid wants to merge 1 commit intoERDDAP:mainfrom
vishwas-droid:fix/ulongarray-reorder-validation

Conversation

@vishwas-droid
Copy link

@vishwas-droid vishwas-droid commented Mar 3, 2026

Description

While reviewing ULongArray, I noticed that reorder() relied on implicit
assumptions about the provided rank array. There were no checks for null
input, length mismatch, or out-of-range indices. In certain edge cases,
this could lead to ArrayIndexOutOfBoundsException or unintended data
corruption.

This change introduces explicit validation to ensure the rank array
matches the current size and contains only valid indices. The method now
fails fast with clear error messages, improving defensive correctness.

I also added an early-return guard in subset() when size == 0 to make the
control flow clearer and easier to reason about. Additionally, the class
documentation now clarifies that ULongArray is not thread-safe due to its
mutable internal state.

These updates do not change behavior for valid inputs and require no
additional dependencies. The goal is to strengthen robustness and improve
long-term maintainability.

Type of change

  • Bug fix (non-breaking change which fixes an issue)

Checklist before requesting a review

  • I have performed a self-review of my code
  • My code follows the style guidelines of this project
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings
  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes

@ChrisJohnNOAA
Copy link
Contributor

This looks to me like it will be slower and with minimal improvement for actual usage (for example if the rank array is invalid most likely it will throw an index out of range exception already).

Is this solving a problem, or just a theoretical problem that AI picked?

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