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

NPE prevention for new moves #545

Open
wants to merge 2 commits into
base: develop
Choose a base branch
from
Open

NPE prevention for new moves #545

wants to merge 2 commits into from

Conversation

syrusgreycloak
Copy link

Added null checks and default return values so current iteration of program will still start and load known pokemon even if moves are unknown.

knickels added 2 commits February 17, 2017 12:13
…gram load when encountering unrecognized moves.
…gram load when encountering unrecognized moves.
@Wolfsblvt
Copy link
Owner

Seems really cool!
Saves me a lot of time, thank you (:

Will review thoroughly soon.

} else {
weaveEnergyUsageRatio = (double) Math.abs(pm2.getEnergy()) / (double) pm1.getEnergy();
}
if (pm1 != null && pm2 != null) {

Choose a reason for hiding this comment

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

Wouldn't a' if ( conditions ) return 0.0;' more simple and legible? Consider this comment on the others validations as well.

Copy link
Author

Choose a reason for hiding this comment

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

It's a style preference. My employer has strict rules on only one return from a method for code maintenance, so it has snuck its way into my coding.

@krc1
Copy link

krc1 commented Feb 22, 2017

I haven't reviewed the changes in great detail but I've applied the changes and I've been running a couple of days now without any issues so far, fwiw.

@Wolfsblvt
Copy link
Owner

Heya. Can you resolve the current conflicts after the PR adding Gen 2 got merged?
Then I can merge yours.

@Wolfsblvt Wolfsblvt modified the milestones: v0.1.8, v0.1.7 Feb 23, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants