buildingSMART / NextGen-IFC

61 stars 4 forks source link

IfcMaterialLayerSet => IfcMaterialLayerList and other Set/List inconsistencies #29

Open jakob-beetz opened 4 years ago

jakob-beetz commented 4 years ago

Tiny, and maybe discussed before within MSG but I have witnessed people being infuriated slightly irritated about this: Currently, we have

ENTITY IfcMaterialLayerSet
 SUBTYPE OF (IfcMaterialDefinition);
    MaterialLayers : LIST [1:?] OF IfcMaterialLayer;
    LayerSetName : OPTIONAL IfcLabel;
    Description : OPTIONAL IfcText;
 DERIVE
    TotalThickness : IfcLengthMeasure := IfcMlsTotalThickness(SELF);
END_ENTITY;

with LIST as a collection type (proper, because order of layers is very relevant) but Set in the naming

similar for

ENTITY IfcPolygonalFaceSet
    Faces : LIST [1:?] OF IfcIndexedPolygonalFace;

I thought this was an inheritance from ISO 10303-42, but there we only have a (meaningful)

EBTITY connected_face_set 
 SUPERTYPE OF (OBEOF (closed_shell, open_shell)) 
  SUBTYPE OF (topological_representation_item); 
      cfs_faces : SET [1:?) OF face; 
END_ENTITY; 
pipauwel commented 4 years ago

+1 seems like an easy fix.

If possible, we should resort to SETs (no order), instead of LISTs. Not possible in this case, so that should be IfcMaterialLayerList and IfcPolygonalFaceList.

TLiebich commented 4 years ago

while I agree on the issue, I would question the cost / benefit ratio here. Yes, a name "IfcMaterialLayerList" is actually more precise, but the cost to change the class = all instance names is high (this entity exists since end of the 1990's in IFC with probably millions of instances in IFC files).

I would opt for not changing, but clearly stating the anomaly in the documentation.

aothms commented 4 years ago

or alternatively:

There are other points to consider in attribute names, such as OffsetFromReferenceLine (a) it's probably a surface at least in case of slabs (b) it doesn't need to be a line, can be an arbitrary curve in case of curved walls. Point is, if we intend to be precise about terminology we need to review a lot.

berlotti commented 4 years ago

Meets the requirements of https://github.com/buildingSMART/NextGen-IFC/wiki/Ten-principles-for-a-future-IFC

Moult commented 4 years ago

What was the conclusion on this one? It wasn't clear from reading the comments.