Re: [Geotools-devel] Encoding Filter.NONE and Filter.ALL

[Ian S. <Ian...@ar...>]

Chris,

>So I decided to implement SQLEncoder to be able to >handle Filter.NONE
and Filter.ALL.

>They're easy to encode, just a TRUE and a FALSE
>Anyone know the status with ALL and NONE filters in the OGC Filter
Encoding Specification?  
>It might be good for us to push to get those in, so we can encode to
XML as well.

No clue on the OGC side, but I will modify the expression parser (it
already has true and false in the grammar, but does not create anything
filter-wise) to produce the appropriate filter.

Some design issues:

>but they don't fit in the visitor pattern all that
>well, since they don't have their own interface.  

This is something I wonder about, as some filters need their own
interface (LikeFilter for example) but others  are really a binary
filter and the interface is used more for tagging. Example:

GeometryFilter
--------------

addRightGeometry(Expression)
addLeftGeometry(Expression)
Expression getRightGeometry
Expression getLeftGeometry

CompareFilter
-------------
addLeftValue(Expression)
addRightValue(Expression)
Expression getLeftValue
Expression getRightValue

So, a BinaryFilter (interface) simply has a:
getLeftExpression
getRightExpression

This leaves the question of how we identify filters. For evaluation
purposes, there is no need to actually know what you are dealing with,
but for encoding or UI purposes, the identification of the filter
describes the behavior.

>I end up just checking visit(Filter filter) for getFilterType()=
(+/-)12345.  
>This feels a bit hacky to me, so I'm wondering if we should give them
interfaces?  
>And/or at least names for their short types in 
AbstractFilter?  

Ah ha, using an integer for types and desiring an associated name -
looks like an ideal candidate for the type-safe enumeration pattern.
This would allow us to remove the visitor pattern, requiring instead
that client code examines the type (as it currently is now for certain
filters) yet tightly couples the short filterType with the common name
of the filter. We can also remove the confused AbstractFilter and the
misnamed (and equally confused) DefaultExpression (which is abstract)
migrating the static functionality they contain, i.e.
isLogicFilter(short expressionType) into the type safe enumerators,
where it belongs.

something along the lines of

interface Filter {
  ...
  FilterType getFilterType();
} 

final class FilterType {
  private final static int LOGIC_BASE = 0xf000;
  public static final FilterType OR = new FilterType(LOGIC_BASE + 1,
"Or");
  public static final FilterType AND = new FilterType(LOGIC_BASE + 2,
"And");
  public static final FilterType NOT = new FilterType(LOGIC_BASE + 3,
"Not");
  ... more static types

  public boolean isLogicFilter() {
    return LOGIC_BASE == (LOGIC_BASE & id);
  }
  ... more meta-data

  public final int id;
  public final int name;
  private FilterType(int id,String name) {
    this.id = id;
    this.name = name;
  }
  
}

walking through a filter/expression tree is not much different without
the visitor pattern then when using the type-safe enumerations.

void encode(Filter f) {
  switch (f.getFilterType().id) {
    case FilterType.AND.id:
      // encode as and
  }
  // now visit children

  if (f instanceof BinaryFilter) {
    BinaryFilter b = (BinaryFilter) f;
    encode(b.getLeftExpression());
    encode(b.getRightExpression());
  }
}

void encode(Expression e) {
  ...
}

If we wanted to be real friendly to filter visitors, we could right a
class which provides this depth first iteration.

>I'm going to commit what I have, but would like to redo this.  Of
course now that data-exp is starting to 
>come into it's own, and we've done feature-exp, it may be time soon for
filter-exp, as it could use a number
> of improvements.  

Ok, agreed.

>I have nothing radical in mind, just some better factories and clean
stuff up.

Yes, my recent experiences with this module have left much to be desired
like API consitency and intuition. I think that the immutability bit can
be taken care of by some nicer factory apis (though I don't think the
mutable factories approach would be a good one here).

>Though others may have better ideas.

See above.  

>We could also consider the continued destruction of defaultcore, and
move all the implementation filter
>stuff to the filter module.  

Whatever everyone wants...

Regards,
Ian