Eric Dobbs wrote:

> Hashtable is a poor tool for modeling SQL.  A Hashtable
> is a one-to-one map of keys and values.  SQL doesn't
> fit that structure.  I see Criterion as a symptom that
> Hashtable is the wrong tool for the job.
>
> Criteria uses column names for keys.  But nested
> Criterion can include various column names.  So which
> column name is the right one for use with the key?
> You could surely establish rules and conventions about
> how to handle that situation, but the *need* for those
> rules is an indication that Hashtable doesn't do what
> an SQL statement needs. 

Point taken.  How does the existing code handle this situation?  Under 
what circumstances do we want to retrieve individual criterion via their 
column name?

>>> 3.  Criterion is ugly, and I wish client code didn't even know
>>>     about it.
>>
>>
>> Ugliness is in the eye of the beholder :-)
>
>
> Behold!
>
> Item 1. An example from the Criteria HOWTO
> Criteria criteria = new Criteria();
> criteria.add(InvoicePeer.COST,
>              1000,
>              Criteria.GREATER_EQUAL);
> Criteria.Criterion criterion =
>     criteria.getCriterion(InvoicePeer.COST);
> criterion.and(
>                criteria.getNewCriterion(
>                              criterion.getTable(),
>                              criterion.getColumn(),
>                              new Integer(5000),
>                              Criteria.LESS_EQUAL )
>                ); 

We could rewrite the above

Criteria criteria = new Criteria();
Criterion x_crit1 = criteria.getNewCriterion(InvoicePeer.COST,new 
Integer(1000),Criteria.GREATER_EQUAL);
Criterion x_crit2 = criteria.getNewCriterion(InvoicePeer.COST,new 
Integer(5000),Criteria.LESS_EQUAL);
criteria.add(x_crit1.and(x_crit2));

which is easier to understand, but I take your point.

>
> Item 2. The equivalent SQL:
> WHERE INVOICE_COST >= 1000
>   AND INVOICE_COST <= 5000
>
>
> I want the Criteria object to be as readable as item 2,
> or as close to that as possible within the confines of
> Java syntax.  The code I proposed last year was
> something like this:
> Sql sql = SqlFactory.getInstance("MySQL");
> String where = sql.where(
>     sql.and(sql.greaterEqual(INVOICE_COST,1000),
>             sql.lessEqual(INVOICE_COST,5000) ));
>
> Which is not totally readable, but much better than
> what Criteria currently offers. 

Hmm.  well simple changes could remove the requirement for the new 
Integer() operations, giving us:

Criteria criteria = new Criteria();
Criterion x_crit1 = 
criteria.getNewCriterion(InvoicePeer.COST,1000,Criteria.GREATER_EQUAL);
Criterion x_crit2 = 
criteria.getNewCriterion(InvoicePeer.COST,5000,Criteria.LESS_EQUAL);
criteria.add(x_crit1.and(x_crit2));

which seems at least reasonably beautiful to me.  I mean call me a 
pragmatist, but my main goal right now is to try and correct the 
contradiction between the Torque documentation and the actually 
functioning of the system.  Either through applying the patch, or 
changing the documentation.

> I think those fixes would extend the life of Criteria
> a lot, but I'd still want to replace it. 

Fair enough.  I mean given that this hashtable functionality of the 
Criteria object is actually used, it does seem like there is room for 
confusion.  How much confusion depends on how extensively the ability to 
retrive individual criterion via column name is used, which I would like 
to hear more about.

In the meantime, is there some reason why we can't update the torque 
documentation to indicate that people can't currenly create complex 
nested queries?

I mean we are waiting on .... other people's opinions on my patch, along 
with possible thoughts about a testing strategy. (find testing class 
attached - I'm not saying its great, but I would like to move forward to 
a position where the patch could be applied).

In the meantime, given that it will take time to apply this patch is 
there anything stopping us from adjusting the documentation to make sure 
that no more hapless developers are misled?

CHEERS> SAM
package org.apache.torque.util;

/* ====================================================================
 * The Apache Software License, Version 1.1
 *
 * Copyright (c) 2001 The Apache Software Foundation.  All rights
 * reserved.
 *
 * Redistribution and use in source and binary forms, with or without
 * modification, are permitted provided that the following conditions
 * are met:
 *
 * 1. Redistributions of source code must retain the above copyright
 *    notice, this list of conditions and the following disclaimer.
 *
 * 2. Redistributions in binary form must reproduce the above copyright
 *    notice, this list of conditions and the following disclaimer in
 *    the documentation and/or other materials provided with the
 *    distribution.
 *
 * 3. The end-user documentation included with the redistribution,
 *    if any, must include the following acknowledgment:
 *       "This product includes software developed by the
 *        Apache Software Foundation (http://www.apache.org/)."
 *    Alternately, this acknowledgment may appear in the software itself,
 *    if and wherever such third-party acknowledgments normally appear.
 *
 * 4. The names "Apache" and "Apache Software Foundation" and
 *    "Apache Turbine" must not be used to endorse or promote products
 *    derived from this software without prior written permission. For
 *    written permission, please contact [EMAIL PROTECTED]
 *
 * 5. Products derived from this software may not be called "Apache",
 *    "Apache Turbine", nor may "Apache" appear in their name, without
 *    prior written permission of the Apache Software Foundation.
 *
 * THIS SOFTWARE IS PROVIDED ``AS IS'' AND ANY EXPRESSED OR IMPLIED
 * WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED WARRANTIES
 * OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE
 * DISCLAIMED.  IN NO EVENT SHALL THE APACHE SOFTWARE FOUNDATION OR
 * ITS CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
 * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
 * LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF
 * USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND
 * ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY,
 * OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT
 * OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF
 * SUCH DAMAGE.
 * ====================================================================
 *
 * This software consists of voluntary contributions made by many
 * individuals on behalf of the Apache Software Foundation.  For more
 * information on the Apache Software Foundation, please see
 * <http://www.apache.org/>.
 */

import junit.framework.Test;
import junit.framework.TestSuite;

import org.apache.torque.BaseTestCase;

/**
 * Test class for Criteria.
 *
 * @author <a href="mailto:[EMAIL PROTECTED]";>Christopher Elkins</a>
 * @version $Id: CriteriaTest.java,v 1.4 2001/11/08 13:11:33 mpoeschl Exp $
 */
public class CriteriaTest extends BaseTestCase
{
    private Criteria c;

    /**
     * Creates a new instance.
     */
    public CriteriaTest(String name)
    {
        super(name);

        c = new Criteria();
    }

    /**
     * Creates a test suite for this class.
     *
     * @return A test suite for this class.
     */
    public static Test suite()
    {
        return new TestSuite(CriteriaTest.class);
    }

    /**
     * Test basic adding of strings.
     */
    public void testAddString()
    {
        final String table = "myTable";
        final String column = "myColumn";
        final String value = "myValue";


        // Add the string
        c.add(table, column, (Object)value);

        // Verify that the key exists
        assertTrue(c.containsKey(table, column));

        // Verify that what we get out is what we put in
        assertTrue(c.getString(table, column).equals(value));

        final String table2 = "myTable2";
        final String column2 = "myColumn2";
        final String value2 = "myValue2";
        
        final String table3 = "myTable3";
        final String column3 = "myColumn3";
        final String value3 = "myValue3";
        
        final String table4 = "myTable4";
        final String column4 = "myColumn4";
        final String value4 = "myValue4";
        
        final String table5 = "myTable5";
        final String column5 = "myColumn5";
        final String value5 = "myValue5";

        // Add another string
        Criteria.Criterion x_crit2 = c.getNewCriterion(table2, column2, 
(Object)value2, Criteria.EQUAL);        
        Criteria.Criterion x_crit3 = c.getNewCriterion(table3, column3, 
(Object)value3, Criteria.EQUAL);        
        Criteria.Criterion x_crit4 = c.getNewCriterion(table4, column4, 
(Object)value4, Criteria.EQUAL);        
        Criteria.Criterion x_crit5 = c.getNewCriterion(table5, column5, 
(Object)value5, Criteria.EQUAL); 
        
        
        x_crit2.and(x_crit3).or(x_crit4.and(x_crit5));
        
        Criteria.Criterion x_crit6 = c.getNewCriterion(table2, column2, 
(Object)value2, Criteria.EQUAL);        
        Criteria.Criterion x_crit7 = c.getNewCriterion(table3, column3, 
(Object)value3, Criteria.EQUAL);        
        Criteria.Criterion x_crit8 = c.getNewCriterion(table4, column4, 
(Object)value4, Criteria.EQUAL);        
        Criteria.Criterion x_crit9 = c.getNewCriterion(table5, column5, 
(Object)value5, Criteria.EQUAL); 
        
        
        x_crit6.and(x_crit7).or(x_crit8).and(x_crit9);
        // should make sure we have tests for all possibilities
        
        //assertTrue(x_crit2.equals(x_crit6));   
        
        Criteria.Criterion[] x_crita = x_crit2.getAttachedCriterion();
        
        assertTrue(x_crit2.equals(x_crita[0]));
        assertTrue(x_crit3.equals(x_crita[1]));
        assertTrue(x_crit4.equals(x_crita[2]));
        assertTrue(x_crit5.equals(x_crita[3]));
        
        String[] x_tables = x_crit2.getAllTables();
               
        assertTrue(x_crit2.getTable().equals(x_tables[0]));
        assertTrue(x_crit3.getTable().equals(x_tables[1]));
        assertTrue(x_crit4.getTable().equals(x_tables[2]));
        assertTrue(x_crit5.getTable().equals(x_tables[3]));
        
        
        assertTrue(x_crit2.hashCode() == x_crit2.hashCode());
        assertTrue(x_crit2.toString().equals(x_crit2.toString()));
        //System.out.println(x_crit2.toString());

        Criteria.Criterion x_crit10 = c.getNewCriterion(table2, column2, 
(Object)value2, Criteria.EQUAL);        
        Criteria.Criterion x_crit11 = c.getNewCriterion(table3, column3, 
(Object)value3, Criteria.EQUAL);        
        Criteria.Criterion x_crit12 = c.getNewCriterion(table4, column4, 
(Object)value4, Criteria.EQUAL);        
        Criteria.Criterion x_crit13 = c.getNewCriterion(table5, column5, 
(Object)value5, Criteria.EQUAL); 
        
        
        System.out.println(x_crit10.and(x_crit11.or(x_crit12)).and(x_crit13));
        try{
        Criteria.Criterion x_p_crit = c.parseCriterionString((x_crit10.toString()));
        System.out.println(x_p_crit);
        assertTrue(x_crit10.equals(x_p_crit));
        
        c = new Criteria();
        System.out.println("");
        System.out.println("");
        System.out.println("Trying to handle x_crit2");
        System.out.println("x_crit2 " + x_crit2);
        
        x_p_crit = c.parseCriterionString((x_crit2.toString()));
        System.out.println(x_p_crit);
        assertTrue(x_crit2.equals(x_p_crit));
        
        c = new Criteria();
        System.out.println("");
        System.out.println("");
        System.out.println("Trying to handle x_crit6");
        System.out.println("x_crit6 " + x_crit6);
        x_p_crit = c.parseCriterionString((x_crit6.toString()));
        System.out.println(x_p_crit);
        assertTrue(x_crit6.equals(x_p_crit));
        
        // should make sure we have tests for all possibilities
        }catch(Exception e){e.printStackTrace();}


    }
}

--
To unsubscribe, e-mail:   <mailto:[EMAIL PROTECTED]>
For additional commands, e-mail: <mailto:[EMAIL PROTECTED]>

Reply via email to