I have a problem with this specific bit of code:
if (actorClass != null) {
try {
actor = (Actor) actorClass.newInstance();
} catch (Exception e) {
e.printStackTrace();
System.out.println(e.getMessage());
}
}
return actor;
clearly your intention is to return null here if there's any error. But what happens next is this:
actor = ActorFactory.createActor(type);
actor.display();
... which will of course NPE if theres an error instantiating the Actor. In this case it might seem ok because the NPE comes immediately after the real error gets dumped in the log, but in the wild this kind of code leaves maintainers with an NPE stacktrace that does not help to locate the real problem at all. Returning null on failure in a factory method is *evil*. |
java.net RSS Feeds
