Ticket #787 (closed defect: fixed)

Opened 9 years ago

Last modified 9 years ago

Cannot read FILTER property of LAYER object

Reported by: hrz@… Owned by: mapserverbugs
Priority: low Milestone:
Component: MapScript-PHP Version: 4.2
Severity: minor Keywords:
Cc: sgillies@…

Description

PHP mapscript does not have the equivalent of SWIG mapscript's layer object 
getFilterString() method. One can set the FILTER using $layer->setFilter() but 
cannot retrieve that value.

Change History

Changed 9 years ago by sgillies@…

I moved core code of swig mapscript's layerObj::getFilterString to
msLayerGetFilterString() in layerobject.c.

Changed 9 years ago by dmorissette

  • status changed from new to assigned
Adding this to PHP MapScript now.

Changed 9 years ago by dmorissette

  • cc sgillies@… added
Sean,

I noticed that you called the SWIG MapScript function getFilterString() (bug
288), but IMO this is not consistent with the naming used for other similar
functions such as setFilter(), setProjection() and getProjection().

I'm debating whether the PHP MapScript function should be called
getFilterString() to be consistent with the SWIG API, or getFilter(), to be
consistent with the rest of the current API. Any opinion?

Changed 9 years ago by dmorissette

  • status changed from assigned to closed
  • resolution set to fixed
OK, I have decided to use $layer->getFilter() instead of getFilterString()
because  I found even more prior art: class->getExpression().

The new $layer->getFilter() method has been added to the 4.3 CVS.

Changed 9 years ago by sgillies@…

Daniel,

I used getFilterString because the method returns a string.  At the time, I was
thinking about a getFilter method that could actually return an instance of
expressionObj.

I don't think that it is a heavily used method, so changing it wouldn't be
so bad, but ...

I *do* think that as a general principle, get* methods should return objects.
getLayer() returns layerObj, getClass() returns classObj, etc.  It makes the
API self-describing and more intuitive.

Changed 9 years ago by dmorissette

I think we both agree that there would be good reasons to go either way. If we
were starting from scratch then I agree that getFilterString() would have been
the way to go for the reasons you explain... but since there were already 2
other similar functions that didn't use the get...String() format I thought it
was important to remain consistent within our current API. I guess if we didn't
have a bunch of code using the current API we could afford to change all three
get() functions, but we can't do that without breaking a bunch of code. 
(backwards compatibility again)

BTW, in this specific case we know that we should never need a getFilter() that
returns an expressionObj: if we ever export that object then we would get to it
using layer->expression->property like most other objects. Of course, somebody
already said that nobody would ever need more than 640kb of memory in a PC.

All this being said, should we consider deprecating getExpression() and
getProjection() and creating getExpressionString() and getProjectionString()? We
would keep the old functions there so that old scripts continue to run, but at
least then we can do things "the right way" next time we need a
getSomethingString() method.

Changed 9 years ago by sgillies@…

OK, I agree that getFilterString is really no improvement over getFilter.  
The ideal API would let us do

    s = layer.filter.string

Hmm ... I might even look into whether it would be possible to bind Python
regex objects to layer.filter.regex.  This would let us bypass the lexer
in one more place.

For now I'll add getFilter, getExpression to swig mapscript as new names for
getFilterString and getExpressionString.



Changed 9 years ago by sgillies@…

my pie-in-the-sky idea is bogus, there's no use for regexes in the filter,
only in the expressions.

Note: See TracTickets for help on using tickets.