Use CopyOnWriteArrayList in PrettyTime.

[Isira-Seneviratne]

No description provided.

[lincolnthree]

Hey @Isira-Seneviratne - Are you working on something new? :) Anything I should be aware of?

[Isira-Seneviratne]

Hey @Isira-Seneviratne - Are you working on something new? :) Anything I should be aware of?

Yeah, I used ConcurrentSkipListSet in this PR instead of a volatile list.

[lincolnthree]

Interesting. Can you tell me the reasons you went with this List implementation? I can guess, but I just want to make sure :)

[Isira-Seneviratne]

Interesting. Can you tell me the reasons you went with this List implementation? I can guess, but I just want to make sure :)

It's actually a Set implementation, but it's designed for concurrent access, so a volatile variable is not needed.

Also, IntelliJ recommended using a thread-safe type instead of a volatile field.

[lincolnthree]

Thanks, that helps me understand. There isn't technically anything wrong with volatile here. But I'm sure IntelliJ is scaring people away from it because it's not commonly used, and can be easy to get wrong by accessing non-threadsafe collections or objects across threads, or using the keyword when not necessary.

I would consider this...

Unfortunately, however, this PR breaks the tests (the ones that are passing) for TimeUnit configuration:
https://github.com/ocpsoft/prettytime/blob/master/core/src/test/java/org/ocpsoft/prettytime/PrettyTimeUnitConfigurationTest.java

Note that order of TimeUnits matters, and the default configuration does something a bit strange, providing the "JustNow" TimeUnit: https://github.com/ocpsoft/prettytime/blob/master/core/src/main/java/org/ocpsoft/prettytime/PrettyTime.java#L1647

This TimeUnit is responsible for the default behavior (overriding Milliseconds and Seconds) by default so that anything less than a minute ago/from now shows 'just now' - a more human readable time measure for "short durations".

With your PR, that unit is sorted to somewhere near Minute, and loses priority to Milliseconds and Seconds when calculating non-precise durations - and that's not what the default behavior should be, since the unit needs to be in first place.

If you can figure out a way to do this without breaking the order of the default units, then let's do it. To be honest -- it's been a while since I coded that part of PrettyTime, so it might be worth re-visiting how unit order is preserved and set anyway.

[lincolnthree]

Sorry for the delay on this @Isira-Seneviratne - I will work on it ASAP. Life has gotten a bit unpredictable here.