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.


Log in to reply