Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Get rid of CopyOnWriteArrayList #5346

Conversation

neonowy
Copy link
Contributor

@neonowy neonowy commented Oct 30, 2023

Replaces java.util.concurrent.CopyOnWriteArrayList with a lightweight wrapper around HashSet for storing listeners in a thread-safe way. (See #1892 (comment))

Copy link
Member

@westnordost westnordost left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, thank you!

I figure, all this

    fun addListener(listener: Listener) {
        listeners.add(listener)
    }
    fun removeListener(listener: Listener) {
        listeners.remove(listener)
    }

stuff could be removed now too and replaced with the listeners field to be public, now that it does not expose the whole of a mutable list. (I.e. users call someController.listeners.add(...))

To be clean, it would be necessary however to change Listeners into an interface and make a class like HashSetListeners or whatever that implements Listeners so that users of Listeners don't have access to the forEach method.

So, not sure if it would be worth it 🤷, just thinking aloud. This PR is approved, anyway.

@westnordost westnordost merged commit d74a517 into streetcomplete:master Oct 31, 2023
@FloEdelmann FloEdelmann added code cleanup hacktoberfest-accepted pull request that should be treated as eligible for Hacktoberfest event labels Oct 31, 2023
@westnordost westnordost mentioned this pull request Oct 31, 2023
@FloEdelmann

This comment was marked as off-topic.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
code cleanup hacktoberfest-accepted pull request that should be treated as eligible for Hacktoberfest event
Projects
Status: Released
Development

Successfully merging this pull request may close these issues.

None yet

3 participants