GeoTools
  1. GeoTools
  2. GEOT-3045

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

        Hide
        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
        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
        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
        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
        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
        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
          • Votes:
            0 Vote for this issue
            Watchers:
            1 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved: