Condition framework/ludicrous design ....
-
Suppose you would like to write a DAO with a flexible interface to retrieve
objects based on certain conditions. Then of course, representing the condition(s)
as objects and generating some sort of 'where' clause from the condition
could be a valid solution.
But if you can think only in linearly and not in terms of nested or
composite conditions, you would view the following condition
i == 5 AND j == 6 AND k == 5
not as AND ( AND (i == 5, j ==6), k ==5 ) but instead as
'i ==5' 'AND j ==6' 'AND k == 5'
And of course, to pass in this condition to a DAO interface you would need
a collection of conditions like this:
findByCriteria(Collection<Condition> conditions)
instead of simply
findByCriteria(Condition condition)
But then how to represent conditions? Well of course, you would
use a concrete base class Condition which implements all the basic conditions
like this:
public class Condition {
protected String _field;
protected int _operator;
protected RowValue _value;
public final static int OP_EQUALS = 1;
public final static int OP_LARGER_THEN = 2;
public final static int OP_SMALLER_THEN = 3;
public final static int OP_SMALLER_EQUALS_THEN = 4;
public Condition(String aField, int anOperator, RowValue aValue) {
_field = aField;
_operator = anOperator;
_value = aValue;
}
public Condition(Condition aCondition){
_field = aCondition.getField();
_operator = aCondition.getOperator();
_value = aCondition.getValue();
}
public void toWhereClause(StringBuffer aQuery) {
aQuery.append(_field);
switch(_operator) {
case OP_EQUALS:
aQuery.append("=");
break;
case OP_LARGER_THEN:
aQuery.append(">");
break;
case OP_SMALLER_THEN:
aQuery.append("<");
break;
case OP_SMALLER_EQUALS_THEN:
aQuery.append("<=");
break;
}
aQuery.append("?");
}
}
Apart from some little things like (1) using protected data, (2) using a switch
statement instead of inheritance to represent operations, (3) not testing the
operator values passed in for validity, (4) not testing
for the 'default' of the switch statement, and (5) using a concrete base class
instead of an interface, this could be a valid implementation.
To implement the AndCondition, the programmer extended the normal condition
and pass in the Condition to extend. The toWhereClause() method now simply
delegates to the base class method and adds " AND " to the beginning.
public class AndCondition extends Condition {
private Condition _condition;
public AndCondition(Condition aCondition) {
super(aCondition);
_condition = aCondition;
}
public void toWhereClause(StringBuffer aQuery) {
aQuery.append(" AND ");
_condition.toWhereClause(aQuery);
}
}
Ok, just to make sure the programmer did not loose any information, note
that the condition passed in to the AndCondition is passed to the base class
but also stored as member variable just in case (!?).
Now, this is just a simplified version of the actual case we found in a
production system. The Condition base class also contains the following
convenience methods:
// AND aField == aValue
static Condition createAndConditionEquals(String aField, int aValue);
// AND aField == aValue
static Condition createAndConditionEquals(String aField, String aValue);
with of course also some methods for the OrCondition. This adds cyclic
dependences to the list of bad coding practices.
Of course, one could only imagine how to implement conditions such as
i == 5 AND (j ==4 OR k == 5)
No doubt, we would need the Condition class for representing i == 5, the
AndBracketOpenCondition for "AND (j ==4", and the OrBracketCloseCondition for
"k ==5)".
It took me about 1 hour to really understand this ludicrous design so I wanted
to share this with you all. The (variable) names have been changed to protect
the incompetent.
Cheers
Erik
-
Oh. My. Friggin'. Lord.
-dZ.
-
I've seen this sort of thing before; usually written by folks who haven't quite mastered (begun to understand?) OO design, let alone how to use it. This requires the burn-it-to-the-ground-and-start-over approach, and then force whomever wrote the original to walk through the new stuff so they learn how to do it right.
I feel your pain.
-
Wait a minute - why not use a postfix-stack implementation as opposed to, say, a collection which is much more linear (left to right)
-
@lichens said:
Wait a minute - why not use a postfix-stack implementation as opposed to, say, a collection which is much more linear (left to right)
Or maybe some kind of syntax tree...
-
You know, algorithms to express a graph of objects as a grammar or vica versa are pretty popular masters' thesis topics (search on the term "Abstract Syntax Tree") so most of the legwork for has already been done for you on this one...the real WTF is why the wheel was reinvented (and so badly).
A few simple guidelines to make anyone a better programmer:
1) If the problem's already well solved, don't solve it again unless it's absolutely necessary.
2) Don't think you can do better until you know everything it does.
3) Above all else, don't try to be clever. It won't seem that clever when it comes time to maintain.