Opened 20 years ago

Closed 20 years ago

Last modified 20 years ago

#586 closed defect (fixed)

Mapscript get* methods should return objects, not indexes

Reported by: sgillies@… Owned by: sgillies@…
Priority: high Milestone:
Component: MapScript Version: 4.1
Severity: normal Keywords:
Cc:

Description

Methods like layerObj.getShape should return objects, shapeObj in this case,
instead of ints.  PHP-Mapscript has got this right, and we should move this
way as soon as we can.

This will break old scripts, unfortunately.

If a mapserver shape cache is implemented, it's getShape method should
definitely return shapeObj.

Change History (9)

comment:1 by sdlime, 20 years ago

Just to be clear, they don't return indexes they return success or failure.

comment:2 by sgillies@…, 20 years ago

Status: newassigned
Right.

Here's my solution.  To pave the way for this fairly big future API change
I have done this in mapscript.i

#ifdef NEXT_GENERATION_CLASSES
    %newobject getShape;
    shapeObj *getShape(int shapeindex, int tileindex=0) {
    /* This version properly returns shapeObj and also has its
     * arguments properly ordered so that users can ignore the
     * tileindex if they are not accessing a tileindexed layer.
     * See bug 586:
     * http://mapserver.gis.umn.edu/bugs/show_bug.cgi?id=586 */
        int retval;
        shapeObj *shape;
        shape = (shapeObj *)malloc(sizeof(shapeObj));
        retval = msLayerGetShape(self, shape, tileindex, shapeindex);
        return shape;
    }
#else
    int getShape(shapeObj *shape, int tileindex, int shapeindex) {
        return msLayerGetShape(self, shape, tileindex, shapeindex);
    }
#endif

If we define NEXT_GENERATION_CLASSES when we SWIG

  swig -lang -shadow -DNEXT_GENERATION_CLASSES 

Then we get the new and improved layerObj.getShape.

comment:3 by sdlime, 20 years ago

Looks like that will work. I wonder though if we should by default use the new 
code and give folks the opportunity to stay in the past. Force them to set a 
flag for backwards compatibility.

Steve

comment:4 by sgillies@…, 20 years ago

Steve,

I don't to pee off too many Perl mapscript users.  Python users have
been tolerant of an evolving interface, but the Perl folks mostly want
it to just stay still.  Some didn't like the change from 

imageObj.saveImage -> imageObj.save

and won't be big fans of a change to getShape.

Will leave this bug open while I track down other API changes that
can be naturally grouped along with getShape.

comment:5 by sgillies@…, 20 years ago

This feature now controlled by NEXT_GENERATION_API symbol to separate
it from optional new class names (bug 587).

comment:6 by sgillies@…, 20 years ago

Owner: changed from sdlime to sgillies@…
Status: assignednew
Made the same type of change to shapeObj.copy, again enabled only
with the new symbol.

comment:7 by sgillies@…, 20 years ago

Cc: mapserver-bugs@… added
Status: newassigned
In the interest of cleaning up the mapscript API, the NEXT_GENERATION_API
symbol causes the following method names changes

lineObj.add -> lineObj.addPoint
lineObj.get -> lineObj.getPoint
shapeObj.add -> shapeObj.addLine
shapeObj.get -> shapeObj.getPoint

The result is better clarity of the method action and consistency with
other existing method names such as mapObj.getLayer and layerObj.getClass.

Since the current add/get method names were not something that works
with PHP mapscript due to the use of get as an attribute getter, this
renaming does not break compatibility between SWIG and PHP APIs.


comment:8 by sgillies@…, 20 years ago

Resolution: fixed
Status: assignedclosed
we're making an exception for hashTableObj.  Its get method method returns
a copy of the value.

comment:9 by sgillies@…, 20 years ago

Am removing the NEXT_GENERATION_API option from swig mapscript.  Will not be
an option for the 4.4 release.

Note: See TracTickets for help on using tickets.