search

cowtowncoder / java-uuid-generator

Java Uuid Generator (JUG) is a Java library for generating all standard UUID versions (v1 - v7)
Apache License 2.0
730 stars 104 forks source link

Confusing comment wrt synchronization #79

Closed gabrielbalan closed 1 year ago

gabrielbalan commented 1 year ago

The readme is very clear:

Generators are fully thread-safe, so a single instance may be shared among multiple threads.

That being said, I find this comment rather confusing:

    /* As timer is not synchronized (nor _uuidBytes), need to sync; but most
     * importantly, synchronize on timer which may also be shared between
     * multiple instances

since there's no obvious synchronization in that method, it made me think the comment was meant for the caller. I know this is not a javadoc comment, but still ...

cowtowncoder commented 1 year ago

Ok so what is the ask here? README is correct in that Generator implementations are thread-safe for callers.

If the inline code comment may be confusing feel free to suggest (with a PR or comment) here how to improve it, based on how code appears to work.

gabrielbalan commented 1 year ago

The comment in TimeBasedReorderedGenerator looks like a copy-paste artifact of the comment from TimeBasedGenerator, which I tracker to here.

So it looks like 13+ years ago there was a synchronized block right next to the comment and things made sense. Then the synchronization went away (moved inside the timer?) but the comment was preserved.

As far as I can tell, the comment should be dropped from both TimeBasedGenerator TimeBasedReorderedGenerator.

cowtowncoder commented 1 year ago

Removed, agreed comment was non-sensical and should have been removed as part of earlier refactoring.