Here's a sample of the type of code I'm referring to:
Code: Select all
public boolean hasLightSourceType(LightSource.Type lightType) {
if (lightSourceList != null) {
for (ListIterator<AttachedLightSource> i = lightSourceList.listIterator(); i.hasNext();) {
AttachedLightSource als = i.next();
LightSource lightSource = MapTool.getCampaign().getLightSource(als.getLightSourceId());
if (lightSource != null && lightSource.getType() == lightType)
return true;
}
}
return false;
}
Code: Select all
class LightSourceTypeManipulator {
protected boolean boolResult(LightSource ls) {
return (lightSource != null && lightSource.getType() == lightType);
}
public boolean boolProcess(Token t, LightSource.Type lightType) {
if (t.getLightSourceList() != null) {
for (ListIterator<AttachedLightSource> i = t.getLightSourceList().listIterator(); i.hasNext();) {
AttachedLightSource als = i.next();
LightSource lightSource = MapTool.getCampaign().getLightSource(als.getLightSourceId());
if (boolResult(lightSource))
return true;
}
}
return false;
}
}
Code: Select all
public boolean hasLightSourceType(LightSource.Type lightType) {
LightSourceTypeManipulator lstm = new LightSourceTypeManipulator();
return lstm.boolProcess(this, lightType);
}
Now the other -- very similar -- methods can be rewritten to be much simpler:
Code: Select all
public boolean hasGMAuras() {
LightSourceTypeManipulator lstm = new LightSourceTypeManipulator() {
protected boolean boolResult(LightSource ls) {
if (ls != null) {
if (ls.getLightList() != null) {
for (Light light : ls.getLightList())
if (light.isGM())
return true;
}
return false;
}
};
return lstm.boolProcess(this, lightType);
}
Who should be creating the anonymous class object? In the last two code blocks above, I had hasLightSourceType() and hasGMAuras() create the LightSourceTypeManipulator and override the boolResult() method, but perhaps the code that is calling the hasLightSourceType() method should be creating the object and passing it in as a parameter? Then the method would simply invoke the boolProcess() method and not be stuck with the object creation process.
If the same object were used over and over it would obviously be more memory efficient to do the allocation outside and pass in a reference. But it complicates the outside code because it has to know what the algorithm is that it wants executed. And this algorithm seems to me to be tied to overall object (Token, in this case) so I think it should be hidden.
Well, I think I've just talked myself into the right way to handle this: put the object creation inside the method, not in the caller.
I'm going ahead and posting this anyway. Who knows, maybe it will generate some discussion? If not, it's simply the ramblings of one of the devs and becomes lost amongst the noise of the interwebs.