LANG-1504 - Adding labels to split StopWatch feature

[edelgadoh]

Thanks for your contribution to Apache Commons! Your help is appreciated!

Before you push a pull request, review this list:

• Read the contribution guidelines for this project.
• Read the ASF Generative Tooling Guidance if you use Artificial Intelligence (AI).
• I used AI to create any part of, or all of, this pull request.
• Run a successful build using the default Maven goal with mvn; that's mvn on the command line by itself.
• Write unit tests that match behavioral changes, where the tests fail if the changes to the runtime are not applied. This may not always be possible, but it is a best practice.
• Write a pull request description that is detailed enough to understand what the pull request does, how, and why.
• Each commit in the pull request should have a meaningful subject line and body. Note that a maintainer may squash commits during the merge process.

I created this new PR that is based on this another one #482 created 5 years ago, since the fork belongs to another DEV it was not possible to cherry-pick it and It seems the previous owner didn't reply anymore.

I'm happy to contribute on this feature, any feedback will be fixed / pushed as soon as I have available time.

[garydgregory]

@edelgadoh
Run 'mvn' locally (no args) to avoid pushing broken builds 😉

[garydgregory]

Jira: https://issues.apache.org/jira/browse/LANG-1504

[edelgadoh]

Hi @garydgregory , yeah, I'm running mvn locally and it works fine in Ubuntu. I also included mvn checkstyle:check since there were some issues related to it.

In the last commit I did, almost all jobs passed except for MacOs 13 JDK 25 that failed https://github.com/edelgadoh/commons-lang-bugfix/actions/runs/18793891189/job/53630024591

with this:

025-10-24T23:10:40.2497050Z [ERROR] Failures: 
2025-10-24T23:10:40.4846280Z [ERROR]   FastDateParser_TimeZoneStrategyTest.testTimeZoneStrategy_TimeZone:93->testTimeZoneStrategyPattern_TimeZone_getAvailableIDs:182 java.text.ParseException: Unparseable date: Horário Padrão: Casablanca: with id = 'Africa/Casablanca', displayName = 'Horário Padrão: Casablanca', locale = null, languageTag = 'pt_AO', isAvailableLocale = true, isLanguageUndetermined = false, timeZone = 

but it's not related to my change, my fork is already synced with master. If I get more time, I'll try to explore this another issue in a different thread.

[garydgregory]

Hello @edelgadoh
I've made this a draft PR. Please switch back to ready when you are ready. TY!

[edelgadoh]

Hi @garydgregory , I fixed the unrelated test and I think now it's ready.

[garydgregory]

Hello @edelgadoh
Thank you for your pull request.
Please rebase on git master.
I have comments scattered throughout the PR.
The most important aspect is that this new split code does not integrate with the existing split code; these two must be made to work in sync. See StopWatch.getSplitDuration() for example.
I've converted this PR back to a draft. You can change to ready when you are done reviewing my comments and are done pushing.

[edelgadoh]

Hi @garydgregory,

• I rebased on git master.
• I have reviewed all your comments and pushed the changes.
• I refactored the new method to be a splitHistory since it captures N consecutives timestamp vs the existing split code that only captures 1 timestamp at a time. I didn't get a way to sync these 2 different codes.

Please let me know if now it's better, thanks!

[garydgregory]

Hello @edelgadoh ( CC @ppkarwasz )

Thank you for your updates.

This PR still doesn't address integration with the existing split APIs. We shouldn't have 2 separate split systems in the same class. The existing APIs are listed below with some initial thoughts. The existing split logic should be reimplemented on top of the new one, and the existing tests should pass if all goes well.

• StopWatch.split(): Adding to the end of the split list seems reasonable.

• StopWatch.unsplit(): Removing from the end of the split list (matching split()).

• StopWatch.getSplitDuration(): "This is the Duration between start and latest split."
  This looks like it should be reimplemented to work from the last split in the list (or should it be the first). We might need additional tests.

• StopWatch.getSplitNanoTime(): "This is the time between start and latest split."

• StopWatch.getSplitTime(): "This is the time between start and latest split."

• StopWatch.toSplitString(): "Gets a summary of the split time that this StopWatch recorded as a string."
  We need to decide if this works with the whole list of just the last one.

• StopWatch.formatSplitTime(): "Formats the split time"
  We need to decide if this works with the whole list of just the last one.

• StopWatch.toString() can change as we see fit since its primary purpose is a debug helper.

[edelgadoh]

Hi @garydgregory , I just refactored to address the integration with the existing split API based on your suggestions, I checked all methods you listed above and now it's synced, I think now it's closer to what is expected, thanks!

[garydgregory]

Hello @edelgadoh

Thank you for your updates.

The integration is not fully tested, for example, the following new test fails:

    @Test
    void testGetSplits() {
        final StopWatch stopWatch = StopWatch.create();
        assertTrue(stopWatch.getSplits().isEmpty());
        stopWatch.start();
        testGetSplits(stopWatch);
        testGetSplits(StopWatch.createStarted());
    }

    private void testGetSplits(final StopWatch watch) {
        assertTrue(watch.getSplits().isEmpty());
        watch.split();
        assertEquals(1, watch.getSplits().size());
        watch.unsplit();
        assertTrue(watch.getSplits().isEmpty());
    }

The failure is:

java.lang.NullPointerException
	at java.base/java.util.Collections$UnmodifiableCollection.<init>(Collections.java:1030)
	at java.base/java.util.Collections$UnmodifiableList.<init>(Collections.java:1303)
	at java.base/java.util.Collections.unmodifiableList(Collections.java:1290)
	at org.apache.commons.lang3.time.StopWatch.getSplits(StopWatch.java:345)
	at org.apache.commons.lang3.time.StopWatchTest.testGetSplits(StopWatchTest.java:226)
	at java.base/java.lang.reflect.Method.invoke(Method.java:566)
	at java.base/java.util.ArrayList.forEach(ArrayList.java:1541)
	at java.base/java.util.ArrayList.forEach(ArrayList.java:1541)

My advice is to initialize the splits list with the instance variable declaration and then use List.clear() to reset the list when needed. The added benefit is that you can then make splits final.

[edelgadoh]

Hi @garydgregory, good catch, I just applied your suggestion and the NPE is fixed.