patternjavaMinor
Nested condition subset of the top one
Viewed 0 times
thetopconditionnestedonesubset
Problem
I've got this code
I don't quite like it because I duplicate the
Double best = nodes.get(edge.getToNodeId());
if (best == null || (best > g+h)) {
nodes.put(edge.getToNodeId(), g+h);
if (best == null) {
open.add(new State(edge.getToNodeId(), g, h, top));
} else {
open.modify(new State(edge.getToNodeId(), g, h, top));
}
}I don't quite like it because I duplicate the
best == null clause. Would it be better in these types of situation to duplicate the nodes.put() instead? Or is there some third, more readable, way without duplicates?Solution
I think it's a matter of taste and preference. In either case, you would have to duplicate something. But, there's something that drew my attention. I don't claim it to be correct or be a solution to your problem. Hard to say without knowing the context and data structures that you use. Anyways, here we go...
I noticed that in both cases, whether
Then, we end up with something like this in the client code:
I noticed that in both cases, whether
best == null or not, you pass the same arguments, though to different methods. I suspect that this checking might occur in many other places as well. So, what if we create a method (e.g. addOrModify()) and move this condition statement to that method. And the method would internally decide whether to add a new State or modify the existing one. Somewhat similar behavior may be observed in the java.util.Set.add(E e) method, where the method checks if the element has already been added to the Set and then returns false without adding it twice...Then, we end up with something like this in the client code:
Double best = nodes.get(edge.getToNodeId());
if (best == null || (best > g+h)) {
nodes.put(edge.getToNodeId(), g+h);
open.addOrModify(new State(edge.getToNodeId(), g, h, top));
}Code Snippets
Double best = nodes.get(edge.getToNodeId());
if (best == null || (best > g+h)) {
nodes.put(edge.getToNodeId(), g+h);
open.addOrModify(new State(edge.getToNodeId(), g, h, top));
}Context
StackExchange Code Review Q#23418, answer score: 5
Revisions (0)
No revisions yet.