We all know its a good idea to avoid using out variables in Java. In almost every case the code can be split up to avoid the situation. Its bad semantics because when reading the code its not obvious which object passed, if any will get mutated. So the reader is forced to open the method to find out what it does. In Robert C Martin’s Clean Code Book he calls this forcing a double take, and suggests some ways to avoid the problem. Rarely you find a case where you can’t avoid it, or to be more accurate, avoiding it leads to worse problems.
Take a look at this interface. If I find this populate method, I am likely to have to open implementations. I don’t know if it populates one map from the other or both maps.
import java.util.Map;
public interface NastyPopulator {
void populate(Map<String, String> driver, Map<String, String> driverHolder);
}
The problem is the semantics of map are that you can read from maps or you can write to them. I think this is a slightly better approach below.
I have used ImmutableMap.Builder. The semantics of this are that it is used to build things. I will call build() on the object later to get a map. Next its an ImmutableMap. So not only am I declaring that the two parameters are likely to be used to build something, that thing will ultimately be Immutable so in effect the signature declares that I am not expected to make modifications later. Finally the method name is unambiguous buildMaps, which pretty much says what its going to do.
import com.google.common.collect.ImmutableMap;
public interface LessNastyPopulator {
void buildMaps(ImmutableMap.Builder<String, String> driver, ImmutableMap.Builder<String, String> driverHolder);
}
Feels like a stronger declaration to me. If ImmutableMap is awkward you could define your own static builder and pass that in, at least your still declaring up front, that this method is builds the two maps. If the method is reading from one map and building others, how about passing an iterator and a builder?
