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

Refactor for ControllerGroup + synchronized #73

Open
wants to merge 13 commits into
base: master
Choose a base branch
from

Conversation

GoToLoop
Copy link

@GoToLoop GoToLoop commented Jul 6, 2016

  • I wasn't expecting almost 1000 lines of code, but I've taken the challenge anyways.
  • Placed localized synchronized () not only for those 2 Vector containers from controllers field, but also for the 2 ArrayList in ControllerGroup class.
  • Mostly for all sections doing iteration on them, but also when their size() changes.
  • Prepended @Override for all inherited methods in ControllerGroup class.
  • Prepended final for all fields which didn't seem they would be re-assigned.
  • Lotsa further cleanups I can't remember I did. :P

* I wasn't expecting almost 1000 lines of code, but I've taken the challenge anyways.
* Placed localized `synchronized ()` not only for those 2 _**Vector**_ containers from _controllers_ field, but also for the 2 __*ArrayList*__ in __*ControllerGroup*__ class.
* Mostly for all sections doing iteration on them, but also when their **size()** changes.
* Prepended `@Override` for all inherited methods in __*ControllerGroup*__ class.
* Prepended `final` for all fields which didn't seem they would be re-assigned.
* Lotsa further cleanups I can't remember I did.  :P
Since __*ArrayList*__ isn't safe like __*Vector*__, I'm also adding `synchronized ()` at places where either of its structure's **size()** changes.
Those 6 methods are: **add()**, **addDrawable()**, **remove()**, **removeDrawable()**, **clear()** and **clearDrawable()**.
Of course, all loops traversing this class still needs to be synchronized externally as well; either over **get()** or **getDrawables()**.
Last word: This patch is just a standalone performant refactoring. It's not obligatory for __*ControllerGroup*__'s previous patch.
ControllerGroup< ? extends ControllerGroup< ? > >
Initialize _**String**_ fields w/ an empty `""`.
`float[]` to `float...`.
@sojamo
Copy link
Owner

sojamo commented Sep 22, 2018

Hi @GoToLoop thank you for looking into this however I wont include your suggestions or fix the issue after I have reviewed and evaluated the concurrency issues. I hope you don't mind I close your pull requests.

@sojamo sojamo added the wontfix label Sep 22, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants