Gone are the times of Pascal and Visual Basic. With Java, you can keep your code, which goes beyond basic algorithmic structure, readable and understandable.
Compare these two identical codes:
public void setParameterStore(final ParameterStore parent)
{
for (final Rule rule : config.getRules())
{
if( ! (rule instanceof RuleBuilder) )
continue;
ParameterizedCallback callback = new ParameterizedCallback()
{
@Override
public void call(Parameterized parameterized)
{
Set<String> names = parameterized.getRequiredParameterNames();
if( ! (rule instanceof RuleBuilder) )
return;
ParameterStore store = ((RuleBuilder) rule).getParameterStore();
for (Map.Entry<String, Parameter<?>> entry : parent)
{
String name = entry.getKey();
Parameter<?> parentParam = entry.getValue();
if (!store.contains(name))
{
store.get(name, parentParam);
continue;
}
Parameter<?> parameter = store.get(name);
for (Binding binding : parameter.getBindings())
{
if (!parentParam.getBindings().contains(binding))
throwRedefinitionError(rule, name);
}
for (Constraint<?> constraint : parameter.getConstraints())
{
if (!parentParam.getConstraints().contains(constraint))
throwRedefinitionError(rule, name);
}
for (Transposition<?> transposition : parameter.getTranspositions())
{
if (!parentParam.getTranspositions().contains(transposition))
throwRedefinitionError(rule, name);
}
if (parentParam.getConverter() != null
&& !parentParam.getConverter().equals(parameter.getConverter()))
throwRedefinitionError(rule, name);
if (parentParam.getValidator() != null
&& !parentParam.getValidator().equals(parameter.getValidator()))
throwRedefinitionError(rule, name);
}
for (String name : names)
{
Parameter<?> parameter = store.get(name, new DefaultParameter(name));
if (parameter instanceof ConfigurableParameter<?>)
((ConfigurableParameter<?>) parameter).bindsTo(Evaluation.property(name));
}
parameterized.setParameterStore(store);
}
private void throwRedefinitionError(Rule rule, String name)
{
throw new IllegalStateException("Subset cannot re-configure parameter [" + name
+ "] that was configured in parent Configuration. Re-definition was attempted at ["
+ rule + "] ");
}
};
Visitor<Condition> conditionVisitor = new ParameterizedConditionVisitor(callback);
new ConditionVisit(rule).accept(conditionVisitor);
Visitor<Operation> operationVisitor = new ParameterizedOperationVisitor(callback);
new OperationVisit(rule).accept(operationVisitor);
}
}
and
public void setParameterStore(final ParameterStore parent)
{
for( int i = 0; i < config.getRules().size(); i++ ) {
Rule rule = config.getRules().get(i);
if( ! (rule instanceof RuleBuilder) )
continue;
ParameterizedCallback callback = new ParameterizedCallbackImpl(rule, parent);
}
}
private static class ParameterizedCallbackImpl implements ParameterizedCallback {
parameterized.setParameterStore(store);
}
Which one do you like more? In the first, the logic nests deeper and deeper and it's hard to keep the context in head. The second abstracts the implementation away and keeps just the name of the class, or, a contract.
Another example:
@SuppressWarnings("unchecked")
public <T extends WindupVertexFrame> T findSingletonVariable(Class<T> type, String name)
{
Iterable<WindupVertexFrame> frames = findVariable(name);
T result = null;
Iterator<WindupVertexFrame> iterator = frames.iterator();
if (iterator.hasNext())
{
Object foundObject = iterator.next();
// Check the type.
if (!type.isAssignableFrom(foundObject.getClass()))
{
throw new IllegalTypeArgumentException(name, type, foundObject.getClass());
}
result = (T) foundObject;
// Check if there's just 1.
if (iterator.hasNext())
{
throw new WindupException("findSingleton called for variable \"" + name
+ "\", but more than one result is present.");
}
}
return result;
}
and
@SuppressWarnings("unchecked")
public <T extends WindupVertexFrame> T findSingletonVariable(Class<T> type, String name)
{
Iterable<WindupVertexFrame> frames = findVariable(name);
if( null == frames )
throw new WindupException("Variable not found: " + name);
Iterator<WindupVertexFrame> iterator = frames.iterator();
if( ! iterator.hasNext() )
return null;
Object obj = iterator.next();
// Check if there's just 1.
if( iterator.hasNext() )
throw new WindupException("More than one frame present "
+ "under presumed singleton variable: " + name);
// Check the type.
if( ! type.isAssignableFrom(obj.getClass()) )
throw new IllegalTypeArgumentException(name, type, obj.getClass());
return (T) obj;
}
Obviously, the second example's logic is clearer and eaiser to follow. The conditions, which can result in returning a special (null) value or throw an exception, are handled as soon as possible, and do not clutter the actual logic of the main task.
Conclusion:
For the sake of readability, try to keep your code flat, in the sense of not nesting loops and conditions too much.
Comments