GeoTools

GeneralEnvelope constructed as isNull

Details

  • Type: Improvement Improvement
  • Status: Closed Closed
  • Priority: Major Major
  • Resolution: Fixed
  • Affects Version/s: 2.6.3
  • Fix Version/s: 2.6.4, 2.7-M0
  • Component/s: referencing
  • Labels:
    None

Description

From email:

Hello there

I found out what seems to be a strange behavior in public GeneralEnvelope(final Envelope envelope) (the constructor that receives another envelope as an argument, GeneralEnvelope.java line 178).

What happens is that it simply throws an exception if the provided Envelope is a null Envelope (that is, [0,0,-1,-1]). And that exception happens because it explicitly calls a checkCoordinates() method that
throws the exception if xmin >= xmax or ymin >= ymax.

So, questions are:

  • Should GeneralEnvelope be "general" enough to handle the null Envelope case? (I'd say it should)
  • How should it manage it? Which comes to the question of what's the real intention when calling that checkCoodinates() method.

Cheers
Milton

Activity

Ascending order - Click to sort in descending order
Hide
Permalink
Jody Garnett added a comment -
Change amounted to this:

            if (checkNullCoordinates(ordinates)) {
                return; // null is okay
            }
            checkCoordinates(ordinates);
Show
Jody Garnett added a comment - Change amounted to this:             if (checkNullCoordinates(ordinates)) {                 return; // null is okay             }             checkCoordinates(ordinates);
Hide
Permalink
Milton Jonathan added a comment -
Hey Jody
Didn't quite get it about this change.. And actually it doesn't seem to work: now we checkNullCoordinates() returning true for a null envelope, and then the same exception is thrown by checkCoordinates(), as it did before..
Actually, the checkNullCoordinates() implementation seemed a little odd: if it does NOT find a NaN, it returns false; otherwise, it returns true.. Is that right? Other than that, I didn't see what NaN has to do with our problem at hand..
Show
Milton Jonathan added a comment - Hey Jody Didn't quite get it about this change.. And actually it doesn't seem to work: now we checkNullCoordinates() returning true for a null envelope, and then the same exception is thrown by checkCoordinates(), as it did before.. Actually, the checkNullCoordinates() implementation seemed a little odd: if it does NOT find a NaN, it returns false; otherwise, it returns true.. Is that right? Other than that, I didn't see what NaN has to do with our problem at hand..
Hide
Permalink
Jody Garnett added a comment -
Well I think that may just be it; I need a test case that can reproduce the problem.

I *did* discover and fix a problem with "null" envelopes (ie envelopes where the isNull method returns true).
Show
Jody Garnett added a comment - Well I think that may just be it; I need a test case that can reproduce the problem. I *did* discover and fix a problem with "null" envelopes (ie envelopes where the isNull method returns true).
Hide
Permalink
Milton Jonathan added a comment -
Hello again. OK, this is a test case that can be added to GeneralEnvelopeTest and throws an exception:
    @Test
    public void testConstructionJTSNullEnvelope() {
     new GeneralEnvelope(new double[]{0,0}, new double[]{-1,-1});
    }

As you may notice, those coordinates are the ones given for a JTS or ReferencedEnvelope null envelope. Now, I've finally realized that GeneralEnvelope's concept of "null" envelopes is quite different from those used in JTS' Envelope and ReferencedEnvelope:
- JTS/ReferencedEnvelope.isNull() = ReferencedEnvelope.isEmpty() = GeneralEnvelope.isEmpty() and is given by maxx < minx
- GeneralEnvelope.isNull() is given by having all ordinates equal to Double#NaN (!)
Show
Milton Jonathan added a comment - Hello again. OK, this is a test case that can be added to GeneralEnvelopeTest and throws an exception:     @Test     public void testConstructionJTSNullEnvelope() {      new GeneralEnvelope(new double[]{0,0}, new double[]{-1,-1});     } As you may notice, those coordinates are the ones given for a JTS or ReferencedEnvelope null envelope. Now, I've finally realized that GeneralEnvelope's concept of "null" envelopes is quite different from those used in JTS' Envelope and ReferencedEnvelope: - JTS/ReferencedEnvelope.isNull() = ReferencedEnvelope.isEmpty() = GeneralEnvelope.isEmpty() and is given by maxx < minx - GeneralEnvelope.isNull() is given by having all ordinates equal to Double#NaN (!)
Hide
Permalink
Jody Garnett added a comment -
Test case supplied so I can now reproduce the problem as reported.
Show
Jody Garnett added a comment - Test case supplied so I can now reproduce the problem as reported.
Hide
Permalink
Jody Garnett added a comment -
Closed as of -r35269; thanks for the test example - think I was slow on the uptake.
Show
Jody Garnett added a comment - Closed as of -r35269; thanks for the test example - think I was slow on the uptake.

People

  • Assignee:
    Jody Garnett
    Reporter:
    Jody Garnett
Vote (0)
Watch (1)

Dates

  • Created:
    Updated:
    Resolved: