[OpenLayers-Dev] removeComponent from LinearRing seems to remove too much points

[Fabian Patzke 2]

Hi,
yesterday I stumbled over a problem while writing a function to remove
points from geometries. When I want to remove a point from a LinearRing that
I created with 3 points (the ring will automatically have 4 points to be
closed) I can remove one of them because of the conditions in
OpenLayers.Geometry.LinearRing. Afterwards the LinearRing only consists of 2
individual points (actual 3) and is no longer a ring.

LinearRing states at the moment:

removeComponent: function(point) {
        var removed = this.components && (this.components.length > 3);
        if (removed) {
...

But it must be:

removeComponent: function(point) {
        var removed = this.components && (this.components.length > 3);
        if (removed) {
...

Also there seems to be a bug in the way what is returned by
OpenLayers.Geometry.Collection.removeComponent. It doesn't not really return
if a component existed in that Collection and could really me removed. It
just returns true.

But when it is written like this it returns the correct boolean:
removeComponent: function(component) {
var length = this.components.length;
    OpenLayers.Util.removeItem(this.components, component);

    if (length != this.components.length) {
        // clearBounds() so that it gets recalculated on the next call
        // to this.getBounds();
        this.clearBounds();
        return true;
    }
    return false;
},

I hope you can follow me, if wanted I can create tickets and patch files.
Providing that everything I have written is correct.

So the following (jasmine-1.2.0 test) should be correct, at the moment it
wouldn't work.

describe("OpenLayers.Geometry.LinearRing.removeComponent", function() {
    it("3 point ring", function() {
        var point1 = new OpenLayers.Geometry.Point(1.0, 1.0);
        var point2 = new OpenLayers.Geometry.Point(1.0, 2.0);
        var point3 = new OpenLayers.Geometry.Point(2.0, 2.0);
        var ring = new OpenLayers.Geometry.LinearRing([ point1, point2,
point3 ]);
        var clone = ring.clone();
        expect(clone.equals(ring)).toBe(true);

        var removed = clone.removeComponent(clone.components[0]);
        expect(removed).toBe(false);
        expect(clone.equals(ring)).toBe(true);
    });
    it("4 point ring", function() {
        var point1 = new OpenLayers.Geometry.Point(1.0, 1.0);
        var point2 = new OpenLayers.Geometry.Point(1.0, 2.0);
        var point3 = new OpenLayers.Geometry.Point(2.0, 2.0);
        var point4 = new OpenLayers.Geometry.Point(2.0, 1.0);
        var ring = new OpenLayers.Geometry.LinearRing([ point1, point2,
point3, point4 ]);
        var clone = ring.clone();
        expect(clone.equals(ring)).toBe(true);

        var expectedResult = new OpenLayers.Geometry.LinearRing([ point2,
point3, point4 ]);
        expect(clone.equals(expectedResult)).toBe(false);

        var removed = clone.removeComponent(clone.components[0]);
        expect(removed).toBe(true);
        expect(clone.equals(ring)).toBe(false);
        expect(clone.equals(expectedResult)).toBe(true);
    });
    it("remove non existend point", function() {
        var point1 = new OpenLayers.Geometry.Point(1.0, 1.0);
        var point2 = new OpenLayers.Geometry.Point(1.0, 2.0);
        var point3 = new OpenLayers.Geometry.Point(2.0, 2.0);
        var point4 = new OpenLayers.Geometry.Point(2.0, 1.0);
        var ring = new OpenLayers.Geometry.LinearRing([ point1, point2,
point3, point4 ]);
        var clone = ring.clone();
        expect(clone.equals(ring)).toBe(true);

        var removed = clone.removeComponent(new
OpenLayers.Geometry.Point(7.0, 7.0));
        expect(removed).toBe(false);
        expect(clone.equals(ring)).toBe(true);
    });
});



--
View this message in context: http://osgeo-org.1560.n6.nabble.com/removeComponent-from-LinearRing-seems-to-remove-too-much-points-tp5019251.html
Sent from the OpenLayers Dev mailing list archive at Nabble.com.