jwg12 IfcPoint Dim

[aothms]

Move derived Dim attribute up to IfcPoint

[aothms]

--- tmp/a/IFC.exp	2022-08-26 22:55:57.254516200 +0200
+++ tmp/b/IFC.exp	2022-09-05 20:18:27.263500500 +0200
@@ -4757,22 +4757,20 @@
  SUBTYPE OF (IfcDeepFoundationType);
 	PredefinedType : IfcCaissonFoundationTypeEnum;
  WHERE
 	CorrectPredefinedType : (PredefinedType <> IfcCaissonFoundationTypeEnum.USERDEFINED) OR
  ((PredefinedType = IfcCaissonFoundationTypeEnum.USERDEFINED) AND EXISTS (SELF\IfcElementType.ElementType));
 END_ENTITY;
 
 ENTITY IfcCartesianPoint
  SUBTYPE OF (IfcPoint);
 	Coordinates : LIST [1:3] OF IfcLengthMeasure;
- DERIVE
-	 Dim : IfcDimensionCount := HIINDEX(Coordinates);
  WHERE
 	CP2Dor3D : HIINDEX(Coordinates) >= 2;
 END_ENTITY;
 
 ENTITY IfcCartesianPointList
  ABSTRACT SUPERTYPE OF (ONEOF
 	(IfcCartesianPointList2D
 	,IfcCartesianPointList3D))
  SUBTYPE OF (IfcGeometricRepresentationItem);
  DERIVE
@@ -8573,48 +8571,44 @@
  ((PredefinedType = IfcPlateTypeEnum.USERDEFINED) AND EXISTS (SELF\IfcElementType.ElementType));
 END_ENTITY;
 
 ENTITY IfcPoint
  ABSTRACT SUPERTYPE OF (ONEOF
 	(IfcCartesianPoint
 	,IfcPointByDistanceExpression
 	,IfcPointOnCurve
 	,IfcPointOnSurface))
  SUBTYPE OF (IfcGeometricRepresentationItem);
+ DERIVE
+	 Dim : IfcDimensionCount := IfcPointDim(SELF);
 END_ENTITY;
 
 ENTITY IfcPointByDistanceExpression
  SUBTYPE OF (IfcPoint);
 	DistanceAlong : IfcCurveMeasureSelect;
 	OffsetLateral : OPTIONAL IfcLengthMeasure;
 	OffsetVertical : OPTIONAL IfcLengthMeasure;
 	OffsetLongitudinal : OPTIONAL IfcLengthMeasure;
 	BasisCurve : IfcCurve;
- DERIVE
-	 Dim : IfcDimensionCount := BasisCurve.Dim;
 END_ENTITY;
 
 ENTITY IfcPointOnCurve
  SUBTYPE OF (IfcPoint);
 	BasisCurve : IfcCurve;
 	PointParameter : IfcParameterValue;
- DERIVE
-	 Dim : IfcDimensionCount := BasisCurve.Dim;
 END_ENTITY;
 
 ENTITY IfcPointOnSurface
  SUBTYPE OF (IfcPoint);
 	BasisSurface : IfcSurface;
 	PointParameterU : IfcParameterValue;
 	PointParameterV : IfcParameterValue;
- DERIVE
-	 Dim : IfcDimensionCount := BasisSurface.Dim;
 END_ENTITY;
 
 ENTITY IfcPolyLoop
  SUBTYPE OF (IfcLoop);
 	Polygon : LIST [3:?] OF UNIQUE IfcCartesianPoint;
  WHERE
 	AllPointsSameDim : SIZEOF(QUERY(Temp <* Polygon | Temp.Dim <> Polygon[1].Dim)) = 0;
 END_ENTITY;
 
 ENTITY IfcPolygonalBoundedHalfSpace
@@ -13255,20 +13249,40 @@
    END_LOCAL;
      N := SIZEOF (APath.EdgeList);
    REPEAT i := 2 TO N;
       P := P AND (APath.EdgeList[i-1].EdgeEnd :=:
                   APath.EdgeList[i].EdgeStart);
    END_REPEAT;
    RETURN (P);
 
 END_FUNCTION;
 
+FUNCTION IfcPointDim
+ (Point : IfcPoint)
+ : IfcDimensionCount;
+
+  IF ('IFC4X3_DEV.IFCCARTESIANPOINT' IN TYPEOF(Point))
+    THEN RETURN(HIINDEX(Point\IfcCartesianPoint.Coordinates));
+  END_IF;
+  IF ('IFC4X3_DEV.IFCPOINTBYDISTANCEEXPRESSION' IN TYPEOF(Point))
+    THEN RETURN(Point\IfcPointByDistanceExpression.BasisCurve.Dim);
+  END_IF;
+  IF ('IFC4X3_DEV.IFCPOINTONCURVE' IN TYPEOF(Point))
+    THEN RETURN(Point\IfcPointOnCurve.BasisCurve.Dim);
+  END_IF;
+  IF ('IFC4X3_DEV.IFCPOINTONSURFACE' IN TYPEOF(Point))
+    THEN RETURN(Point\IfcPointOnSurface.BasisSurface.Dim);
+  END_IF;
+  RETURN (?);
+
+END_FUNCTION;
+
 FUNCTION IfcPointListDim
 (PointList : IfcCartesianPointList)
            : IfcDimensionCount;
 
   IF ('IFC4X3_DEV.IFCCARTESIANPOINTLIST2D' IN TYPEOF(PointList))
     THEN RETURN(2);
   END_IF;     
   IF ('IFC4X3_DEV.IFCCARTESIANPOINTLIST3D' IN TYPEOF(PointList))
     THEN RETURN(3);
   END_IF;     

[SergejMuhic]

May I propose a case statement here? It is sort of on the boundary (4 ifs + default makes 5 checks), but it would look much better.

[aothms]

But then what about all the other Ifc____Dim functions? This seems to be the way we have done things in the past and it was just copied over by Thomas.

[SergejMuhic]

Not really. Dim functions were typically not separately defined. If you look at something like IfcCorrectDimensions, you can see CASE being used. It makes for a much nicer function.

Let me know whether I should look at it.

[aothms]

But IfcCorrectDimensions checks for enum constants, typeof() returns a set of strings, I don't see how it would work in a case statement.

[SergejMuhic]

Indeed it might not work best. I found a definition of TYPEOF. My idea was to

case TYPEOF(Point)[1]

which would potentially work in this particular case (maybe as [2]) where IfcPoint only has leaf nodes since the set is initialized with the type that Point was declared as and that the instance represents.

[pjanck]

Meta: How to give approval (this isn't a PR ...)?

I greet the introduction of the function and it seems to have correct syntax.

[aothms]

@SergejMuhic typeof() returns a set; can they be indexed like that and are they ordered to the extent that the first element is the most specific type? It's an honest question, but also hints at my doubts on doing too much refactoring on these rules.

@pjanck this is just a quick and dirty way for @SergejMuhic and me to communicate. I think what will happen is that @SergejMuhic creates a PR off of this discussion.

[pjanck]

just a quick and dirty way

I see. Then I'll await the PR.

[SergejMuhic]

Finally remembered how I wanted to implement CASE for these kinds of checks! It slipped my mind and I couldn't remember anymore. bSI-InfraRoom/IFC-Specification#533 (comment)

CASE TRUE OF
    VALUE_IN(typepath,'IFCGEOMETRYRESOURCE.IFCSPIRAL') :  do_something;

[aothms]

Hm yes, but this doesn't match the typical switch/case expression in typical languages where the switch has an expression, but the cases are statically evaluated (in C languages only integral types even, so no strings, so that it directly translates to a jmp instruction). At least the if statement approach is more universally covered in languages. It's just really unfortunate that this is how type checking is done in express.

[SergejMuhic]

switch/case expression in typical languages where the switch has an expression

Certainly, completely agree there. Technically, TRUE is an expression though. 😉

It would make for a clearer interpretation way for type checks. That way my point. It also looks nicer than a bunch of IFs.