Please review my criteriaFind method

[Sergei Simonov]

Hi there
Please review and comment my criteriaFind method implementation.
fieldMap - HashMap contains field name as the Key and order number of it as the value.

    /**
* Searches the database for entries matching the criteria supplied. 
* Criteria take the form of a comma separated list of <field name>=<value to match> 
* specifications. 
    * @param criteria The string containing criteria.
    */
    public DataInfo[] criteriaFind(String criteria)  throws DatabaseException, IllegalArgumentException
    {
    if (criteria == null)
    throw new IllegalArgumentException("Search criteria string is null");
    
    // Check database integrity
    invariant();

    DataInfo[] rv = new DataInfo[0];
    ArrayList fetchedRecords = new ArrayList();
    StringTokenizer parser = new StringTokenizer(criteria, "=',");
String strFieldNameToken = null, strFieldValueToken = null;
TreeMap searchableFields = new TreeMap();

    while(parser.hasMoreTokens())
    {
    // Get field name first
    strFieldNameToken = parser.nextToken();
Integer fieldOrderNumber = (Integer) fieldMap.get(strFieldNameToken);
       // If this field name is not found in the map 
       // of existing fields of the DB schema 
    // then we just return from the method right away without error
    // but with zero length array result
if (fieldOrderNumber == null)
return rv;
    
    // If field name was specified but value for it is absent -
    // then we throw the IllegalArgumentException
    if (!parser.hasMoreTokens())
    throw new IllegalArgumentException("Value was not specified for field " + strFieldNameToken + 
    " in the search criteria string");
    
    // Get field value second
    strFieldValueToken = parser.nextToken();
    searchableFields.put(fieldOrderNumber, strFieldValueToken);
    }
    Set searchableFieldsSet = searchableFields.entrySet();
    
    // Now go through all records in the DB
    for(int i = 1; i <= getRecordCount(); i++)
    {
    DataInfo rec = getRecord(i);
    boolean isToAdd = true;
    // Go through list of searchable fields
    for ( Iterator iter = searchableFieldsSet.iterator(); iter.hasNext(); )
    {
    Map.Entry entry = (Map.Entry) iter.next();
    int fieldIndex = ((Integer) entry.getKey()).intValue();
    String fieldValue = (String) entry.getValue();
    
    if (fieldValue.equalsIgnoreCase("any"))
    continue;
    
// If at least one field value in the record 
// of the list of searchable non equal searchable value
// then this record should not be included in the final
// list of found records and no need to check other fields
// in this record (exit out of loop)
if (!rec.getValues()[fieldIndex].equals(fieldValue))
{
isToAdd = false;
break;
}
    }
    
    // Add record to list of found ones
    if (isToAdd)
    fetchedRecords.add(rec);
    }
    
    rv = new DataInfo[fetchedRecords.size()];
    rv = (DataInfo[]) fetchedRecords.toArray(rv);
    
    return rv;
    }

[ December 20, 2002: Message edited by: Junilu Lacar ]

[Mag Hoehme]

Hi Sergei,
if it works, it is ok.
However, I find your method somewhat long. I stuffed all this parsing and matching into a separate class, thus encapsulating it.
As a rule of thumb: your methods should be so long that you can view them on the screen without scrolling. If your methods are longer than that, consider refactoring them.
Besides, I used two tokenizers, one for separating the key-value pairs, and the other one for separating the key from the value. Instead of putting all possible delimiters into one string you could use the nextToken(String delim) method.
Hope this helps.