Fix #6563: Fixed In Transit Sorting in Warehouse Tab #6585
Conversation
- Implemented a new `StringAndThenNumberSorter` comparator to handle sorting of strings with embedded numeric values. - Updated `WarehouseTab` to use `StringAndThenNumberSorter` for the `COL_STATUS` column in the parts table. - Enhanced sorting behavior to properly order entries with mixed alphanumeric content, i.e. `In Transit (6 days)` will appear before `In Transit (10 days)`, but both will appear after `Functional`.
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #6585 +/- ##
=========================================
Coverage 11.43% 11.43%
- Complexity 6451 6452 +1
=========================================
Files 1087 1088 +1
Lines 139451 139468 +17
Branches 21560 21563 +3
=========================================
+ Hits 15943 15951 +8
- Misses 121916 121923 +7
- Partials 1592 1594 +2 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
| */ | ||
| private static final Pattern NUMBER_PATTERN = Pattern.compile("(\\d+|\\D+)"); | ||
|
|
||
| /** |
There was a problem hiding this comment.
Pattern is incorrect. that will match all numbers and non-numbers.
To match all numbers it is \d+ (or for that variation \\d+)
There was a problem hiding this comment.
If it's only displayed in days, the \\d+ will work. Otherwise you'll need to adjust it to be \\(((\\d+) (\\D+)\\) to capture the number of days AND interval to compare days, weeks, months, years...
There was a problem hiding this comment.
I'm not sure what I need to do here, could you provide a little more information?
For context, the table column uses a mix of both numbers and non-numbers. For example, the column might have 'damaged', 'functional', 'in transit (5 days)', 'in transit (353 days)'. The intention is that it will sort based on letters and then, when it hits a number, start sorting based on that.
Right now, the implementation visually works correctly, but if I move it to \\d+ I run into Comparison method violates its general contract! errors, presumably because I'm left with empty strings to compare because all the non-digits get filtered out.
There was a problem hiding this comment.
Also, if it helps, it only appears in batches of 'days', we don't convert that to months or similar
…ter`. - Updated the comparator logic sort strings numerically for "day(s)" and alphabetically otherwise. - Renamed and refactored the sorter to align with its updated functionality and naming conventions. - Adjusted the `WarehouseTab` to utilize the new `WarehouseStatusSorter`.
|
… methods and class were introduced. - Included `@author Illiani` annotations for proper attribution.
There was a problem hiding this comment.
Nice!
I think I prefer c872c04. While both currently work, I think the older commit version would be more robust. The older commit implementation will natively support translations as long as it follows the pattern of . The older commit implementation will better support if a second warehouse part status gets "days" added to it, like days remaining for repairing something broken. I know this contradicts what we discussed yesterday but after comparing the two and taking translation support more seriously I think the original commit will be more robust.
Does anyone else have any thoughts on this?
| * | ||
| * <p>Matches strings like "(5 days)" or "(1 day)" and captures the numeric part.</p> | ||
| */ | ||
| private static final Pattern DAYS_PATTERN = Pattern.compile("\\((\\d+)\\s*day(s)?\\)"); |
There was a problem hiding this comment.
You can make a group a "non-capturing" group by starting it with ?: day(?:s)?. Though in this case you don't need the capture group at all, you should get the same result with days?.
|
StringAndThenNumberSortercomparator to handle sorting of strings with embedded numeric values.WarehouseTabto useStringAndThenNumberSorterfor theCOL_STATUScolumn in the parts table.In Transit (6 days)will appear beforeIn Transit (10 days), but both will appear afterFunctional.Fix #6563
Dev Notes
RegEx is freakin' sorcery, I swear.
I stole this pattern from the internet so don't ask me how it works, I just know it does.