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

Changed large if-else to use a HashMap #7

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

Conversation

mart2967
Copy link

@mart2967 mart2967 commented May 1, 2014

We added a hashmap field to the class, that takes a DamageSource and returns a DamageCause. This HM is instantiated in a discrete method that is called on initialization. The HM eliminates the need for a lengthy if-else series, and simplifies the addition of new damage sources and causes.

map a field of the class that is instantiated by a method
Conflicts:
	src/main/java/org/bukkit/craftbukkit/event/CraftEventFactory.java
@NicMcPhee
Copy link
Owner

Note that the Bukkit folks want pull requests to be on a new branch whose name indicates something about the change being implemented. I can merge this into a new branch on my version, but it would be better if you created a branch on your end, merged these changes to that branch, and then issued the pull request from the new branch.

@NicMcPhee
Copy link
Owner

Nice clean refactoring, although your write-up doesn't really match the CraftBukkit pull request specifications at all.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants