Make getShape part of next generation API official

[tbonfort]

Reporter: sgillies@frii.com Date: 2004/09/20 - 19:00

For some time, an alternative API has been available to to those
who build their own SWIG mapscript modules.  I think it is time
to make one feature of the experimental API permanent.

In the experimental API getShape returns a shapeObj and takes
shapeindex and tileindex as arguments.  

    shapeObj getShape( int shapeindex [, int tileindex=-1 ] )

The usage for the case of tileindexed shapefiles is like this

    tileindexed_layer.open()
    shape = tileindexed_layer.getShape(shapeindex, tileindex)
    tileindexed_layer.close()

In the case of layers without tileindexing, the second argument
can be omitted because it defaults to -1

    layer.open()
    shape = layer.getShape(shapeindex)
    layer.close()

I'd like to get feedback on 2 facets of this proposal.  The way
I see it

1) return value change from int to shapeObj

   requires update of scripts:    -1
   more like php mapscript:       +1
   prevents shape type conflicts: +1
   reduces need to create shapes: +1
   easier to document:            +1
   just plain better:             +1

2) reorder arguments

   less like php mapscript:                  -1
   for postgis layers avoids a useless argument:  +1
   for regular shapefile layers avoids useless:   +1

[tbonfort]

Author: sgillies@frii.com Date: 2004/09/22 - 17:17

Adding y'all for your feedback.  To this point, feedback from the users list
and on #mapserver has been 100% in favor of this change.

[tbonfort]

Author: dmorissette Date: 2004/09/22 - 22:38

I wonder if the people on the users list and IRC realize the impact of this
change on existing scripts.

I'd rather go with getShape(tileindex, shapeindex) for compatibility with PHP
MapScript. I understand the reasons why tileindex could have been optional in
the first place, but it's too late to go back and unfortunately we cannot adjust
PHP MapScript (i.e. flip tileindex and shapeindex) to follow your suggestion
without opening the risk for existing scritps to break for no apparent reason.
If the two arguments had been of different type then we could have used type
validation to produce an error if the script doesn't pass the arguments in the
right order, but that won't be possible in this case. (I'm not sure if I'm
explaining things well here).

We could say that it's OK for PHP to use a different order from SWIG, but that's
just opening the door to confusion in users minds.

BTW, in many cases, we won't benefit that much from the tileindex being optional
since the application code doesn't know whether the layer is tiled or not: it
just reads the tileindex and shapeindex values from the query result cache and
passes them directly to the getShape() call. The fact that tileindex is unused
by postgis and regular shapefiles is not that big a deal IMHO: let's just
declare that there is always a single "universe" tile with index of -1 in those
layers and then the consistency issue is solved.

So I'd vote for either getShape(tileindex, shapeindex) or finding a new name for
the new function and deprecating the old one... if someone can find a nice name
for the new function.

[tbonfort]

Author: dmorissette Date: 2004/09/22 - 22:42

BTW, I think this issue should be brought up on the -dev list. Most of the
developers aren't regularily on IRC, and many of us don't follow the -users list
any more because it has too much traffic.

[tbonfort]

Author: sgillies@frii.com Date: 2004/09/22 - 23:09

Maybe the better thing to do is to implement this as "getFeature".  Has a 
symmetry with the existing "addFeature" method.

[tbonfort]

Author: dmorissette Date: 2004/09/22 - 23:44

It might be a bit odd that getFeature returns a shapeObj, but I could live with
that as a compromise if everybody else agrees. I personally see Features and
Shapes as the same thing, but maybe Steve sees them as meaning different things
in the context of MapServer. Let's see what he thinks.

Adding Frank to the CC as he may have some thoughts too.

[tbonfort]

Author: fwarmerdam Date: 2004/09/23 - 00:01

I agree that changing the old getShape() could be quite disruptive.  Better
to deprecate it, and provide a new method.  Calling it getFeature() is much
better than "getShape2()", which was my first thought.

[tbonfort]

Author: sgillies@frii.com Date: 2004/09/23 - 00:03

Cool.

There's precedence for "feature" in map.h -- layerObj.features
and layerObj.currentfeature.

[tbonfort]

Author: sgillies@frii.com Date: 2004/09/25 - 18:16

Tests pass, documented in mapscript/doc/mapscript.txt. Committed in CVS HEAD.

[tbonfort]

Author: dmorissette Date: 2004/09/26 - 14:31

Created bug 878 about doing the same for PHP MapScript.

[tbonfort]

Author: dmorissette Date: 2004/09/26 - 14:32

Doh!  I meant bug 900!