Function with SDO_GEOMETRY 1

[Andrzejek]

I am in the world totally new to me, so… be kind.

I am trying to write a function in Oracle that would take SDO_GEOMETRY (a line) and return the mid-point of that line in ddmmss format (longitude and latitude). I have it working (I hope it is right) but I would not mind some ‘critic’ of my code. There is always a better way to do stuff. And if anyone finds any miss-calculations in my math, please let me know.

One problem I have is the validation of the passed SDO_GEOMETRY: if it is NOT a line, I would like to have different outcome of that Function, something like ‘not sdo line’

CREATE OR REPLACE FUNCTION S2010000.MID_POINT_OF_LINE 
   ( pass_sdo_geometry IN SDO_GEOMETRY )
   RETURN varchar2
IS
  outcome varchar2(255);  [green] -- I know it should be shorter[/green]
    
  startGeom SDO_GEOMETRY;
  lrsGeom SDO_GEOMETRY;
  measure NUMBER;
  midpoint SDO_GEOMETRY;
  
  x NUMBER;
  y NUMBER;
  
  x_d    CHAR(2 BYTE);
  x_m_no NUMBER;
  x_m    CHAR(2 BYTE);
  x_s    CHAR(2 BYTE);
  
  y_d    CHAR(2 BYTE);
  y_m_no NUMBER;
  y_m    CHAR(2 BYTE);
  y_s    CHAR(2 BYTE);

BEGIN
  startGeom := pass_sdo_geometry;
  
  IF startGeom.SDO_GTYPE = 2002 THEN  [green]-- I don't think this part works[/green]
  
      [green]-- convert standard geometry to LRS geometry[/green]
      lrsGeom := SDO_LRS.CONVERT_TO_LRS_GEOM(startGeom);
  
      [green]-- get the measure of the halfway point of the geometry[/green]
      measure := SDO_LRS.PERCENTAGE_TO_MEASURE(lrsGeom, 50);
  
      [green]-- convert the measure to a point[/green]
      midpoint := SDO_LRS.LOCATE_PT(lrsGeom, measure);
  
      [green]-- convert LRS Lambert point to lat/long point[/green]
      midpoint := SDO_CS.TRANSFORM(midpoint, 8192);
  
      [green]-- extract x/y coordinates from the point[/green]
      SELECT t.x, t.y
      INTO x, y
      FROM dual, TABLE (sdo_util.getvertices (midpoint)) t;

      [green]--outcome := 'x: ' || x || ', y: ' || y;[/green]
      x_d := ABS(TRUNC(x));
      y_d := ABS(TRUNC(y));
 
      [green]--outcome := 'x_d: ' || x_d || ', y_d: ' || y_d;[/green]
 
      x_m_no := SUBSTR(X, 4) * 60;
      x_m := TRUNC(x_m_no);
  
      IF TO_NUMBER(x_m) < 10 then 
         x_m := '0' || x_m;
      END IF;
  
      y_m_no := SUBSTR(y, 3) * 60;
      y_m := TRUNC(y_m_no);
  
      IF TO_NUMBER(y_m) < 10 then 
         y_m := '0' || y_m;
      END IF;  
  
      [green]--outcome := 'x_m: ' || x_m || ', y_m: ' || y_m;[/green]

      x_s := TRUNC((x_m_no - TO_NUMBER(x_m)) * 60);
      IF TO_NUMBER(x_s) < 10 THEN
          x_s := '0' || x_s;
      END IF;
  
      y_s := TRUNC((y_m_no - TO_NUMBER(y_m)) * 60);
      IF TO_NUMBER(y_s) < 10 THEN
          y_s := '0' || y_s;
      END IF;
    
      [green]---outcome := 'x_s: ' || x_s || ', y_s: ' || y_s;[/green]
  
      outcome := x_d || x_m || x_s || ' ' || y_d || y_m || y_s;
  
  ELSE
      outcome := 'not sdo line';
  END IF;
  
RETURN outcome;

END MID_POINT_OF_LINE;
/

I can pass a valid SDO_GEOMETRY for a line and the Function does return ‘954835 404621’ - two longitudes and latitudes of a point.

Have fun.

---- Andy

 

[Andrzejek]

Found a bug in the [tt]IF TO_NUMBER([/tt] statements, need to add TRIM:

IF TO_NUMBER(x_m) < 10 then 
   x_m := '0' || [red]TRIM([/red]x_m[red])[/red];
END IF;

Have fun.

---- Andy

 

[Thargy]

Hi there.

I've just read through your code which seems to be quite good, so I only have a couple of general items to mention.

First of all, if this isn't in a package, create one and put it in there.
Second, in the package specification you should declare some constants to make the code more readable, e.g.

IF startGeom.SDO_GTYPE = 2002 THEN

would become

IF startGeom.SDO_GTYPE = c_latitude_type THEN

Obviously I don't know what the magic number of 2002 represents, but you ought to give it a meaningful name.

You appear to be doing a needless select from dual, why is

      SELECT t.x, t.y
      INTO x, y
      FROM dual, TABLE (sdo_util.getvertices (midpoint)) t;

not written as

      SELECT t.x, t.y
              INTO x, y
              FROM TABLE (sdo_util.getvertices (midpoint)) t;

I can't see any definition for TABLE, so I'm assuming that this represents a table name.

You also appear to be extracting numbers and converting to characters, and then later on doing a lot of TO_NUMBER operations on them.
Just leave them as numbers, do your arithmetic (or whatever) and then convert them to characters once at the end.

Your variable names are too terse for my liking, so I suggest that variables like x_d be given a longer meaningful name, as it will make the code much easier to read.
Finally, this is quite a "big" function with lots of sql calls in it. I suggest that you split some of them out in to separate smaller functions, and then invoke them.
This sounds like a waste of time, but believe me, when it comes time to do maintenance you'll be glad you did.


Regards

T

 

[Andrzejek]

Thanks Thargy, much appreciated.

“if this isn't in a package, create one and put it in there” – this Function works fine the way it is, so what it will do better/different when/if I put it into a package? (that shows my ‘newbeness’ in this subject )

c_latitude_type as constant = 2002 – agreed, that shows my ‘work in progress, fix it later’ approach.

2002 states for a line in SDO_GEOMETRY, there is also a point, polygon, multi-point, multi-line, etc. Just FYI

Select from dual – that part I’ve got from a GIS expert (along all other stuff just before that). It works, it does something, not my invention, so I left it alone (here I show my laziness and seldom opportunity to ‘delegate’ some work to others )

TO_NUMBER conversions, I see your point. Thanks.

Variable names, that’s tough. It is all about longitude / latitude, but here a lot of people refer to it as ‘x and y’ (drives me crazy). That’s why x_d (x degrees), x_m (x minutes), x_s (x seconds) etc.


Have fun.

---- Andy

 

[Thargy]

Andy,

In general, all code should be in a package. This enables encapsulation of the code, and improves security.
It enables the declaration (in the package specification) of constants and variables with restricted scope.
It also enables you to develop a library of associated functions and procedures and keep them all in one place.

It may seem over the top, but I will quite happily create a package which contains just one function.
Although folks will assure you that no more code will be needed, it always is, and the package grows.

As for the variable names, if they are within the package, then their scope is restricted to the package.
That means that you can call x_d, x_m and x_s something like 'latitude_degrees', latitude_minutes and latitude_seconds which, in my opinion, will make the code much easier to read and understand.
Their restricted scope means that they cannot affect or be affected by names outside of the package.

I also noticed the use of the word 'TABLE' in the code. This is a reserved word in oracle, so it should be avoided completely.
It's a bit like creating a function with the name of 'function'. Even if technically possible, it should not be done.

Regards

T

 

[Andrzejek]

Thank you for the pointers.
Will take them under consideration.
Appreciate it.


Have fun.

---- Andy