Created
August 2, 2017 00:39
-
-
Save ErikGranse/8e0a0f8f76822c86c44c6794678d3b0f to your computer and use it in GitHub Desktop.
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
// Analyze the following code snippet for "code smells" | |
// Assume the method below is in a class, and it already compiles successfully | |
// --- | |
/** | |
* Calculates the weight of a person for a given unit on a given planet | |
* | |
*/ | |
public float calculateWeight(int personId, String unit, int planetId) { | |
Person person = personDao.getPerson(personId); | |
Planet planet = planetDao.getPlanet(planetId); | |
float gravitationalConstant = planet.getGravitationalConstant(); | |
if (unit.equals("POUNDS")) { | |
gravitationalConstant = gravitationalConstant * 2.205f; //convert kg to pounds | |
} else if (!unit.equals("KILOGRAMS")) { //an invalid unit was provided | |
log.error("Invalid unit provided"); | |
throw new IllegalArgumentException("Unknown unit provided."); | |
} | |
float weight = person.getMass() * gravitationalConstant; | |
return weight; | |
} |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Yeah, I agree. My description of intent from years ago isn't great.
What I was trying to say is that having a candidate call out implementation details like the potential NPEs is OK, but I really want to see people comment on the terrible fundamental design of this method.