Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Bug]: GetLine() output of Boolean attributes #1257

Open
andy-wrks opened this issue Feb 9, 2025 · 6 comments
Open

[Bug]: GetLine() output of Boolean attributes #1257

andy-wrks opened this issue Feb 9, 2025 · 6 comments
Assignees
Labels
bug Something isn't working

Comments

@andy-wrks
Copy link

What happened?

I'm using GetLine() to read an IfcWindowStyle entity and I realized that the boolean attributes are hard to identify from the returned object.

This is the raw line:
#150288=IFCWINDOWSTYLE('2oBbts7bIYPxtVzht3Nx15',#30,'Kombi-Fenster 1-Fl 22',$,$,$,$,'B22E5DF6-1E54-A267-BDDF-F6BDC35FB045',.NOTDEFINED.,.NOTDEFINED.,.F.,.F.);
Image

I get the following object from GetLine():
Image

The highlighted boolean enumeration values "F" show up differently than the "NOTDEFINED" enumeration values. Is there a reason the booleans don't show up as {type: 3, value: 'F'}? I wasn't expecting a naked string here that's not wrapped in the usual value/type object. It also makes it hard to identify booleans when they're not tagged as an enumeration. It may just be a string 'F', it's hard to know.
It would be great if the IfcBooleans and IfcLogicals would show up consistently like other enumerations.

A related question: I came across objects like {type: 10, value: [1, 2, 3, 4]}. Type=10 stands for Integer. Can this refer to single values as well as an array of Integers? Does this apply to all types? I couldn't find any documentation on this other than the type map in some code file.

Thanks for the great work on web-ifc!

Version

0.0.66

What browsers are you seeing the problem on?

Chrome

Relevant log output

Anything else?

No response

@andy-wrks andy-wrks added the bug Something isn't working label Feb 9, 2025
@andy-wrks andy-wrks changed the title [Bug]: GetLine() output of Booleans attributes [Bug]: GetLine() output of Boolean attributes Feb 9, 2025
@beachtom beachtom self-assigned this Feb 14, 2025
@liradb2000
Copy link
Contributor

This will be fixed when #1289 is merged.

Image

Regarding the {type: 10, value: [1, 2, 3, 4]} question:

When Type is defined as ARRAY OF <Standard Type (e.g., Real, Integer, String...)>,
it will always return { type: , value: [...values...] }.

// EXPRESS specification:
TYPE IfcComplexNumber = ARRAY [1:2] OF REAL;
END_TYPE;

// converted code
export class IfcComplexNumber {
  type: number = 4;
  constructor(public value: Array<number>) {}
}

However, when Type is defined as an IFCType, it will return [...typeClass...]

// EXPRESS specification:
ENTITY IfcBSplineCurve
 ABSTRACT SUPERTYPE OF (ONEOF
	(IfcBezierCurve))
 SUBTYPE OF (IfcBoundedCurve);
	Degree : INTEGER;
	ControlPointsList : LIST [2:?] OF IfcCartesianPoint;
	CurveForm : IfcBSplineCurveForm;
	ClosedCurve : LOGICAL;
	SelfIntersect : LOGICAL;
 DERIVE
	ControlPoints : ARRAY [0:255] OF IfcCartesianPoint := IfcListToArray(ControlPointsList,0,UpperIndexOnControlPoints);
	UpperIndexOnControlPoints : INTEGER := (SIZEOF(ControlPointsList) - 1);
 WHERE
	WR41 : SIZEOF(QUERY(Temp <* ControlPointsList |
               Temp.Dim <> ControlPointsList[1].Dim))
             = 0;
END_ENTITY;

// converted code
export class IfcBSplineCurve extends IfcBoundedCurve {
  type: number = 1967976161;
  constructor(
    public Degree: number,
    public ControlPointsList: (Handle<IfcCartesianPoint> | IfcCartesianPoint)[],
    public CurveForm: IfcBSplineCurveForm,
    public ClosedCurve: logical,
    public SelfIntersect: logical
  ) {
    super();
  }
}

You can find this converted code in src/ts/ifc-schema.ts.

I hope this comment is helpful.

@andy-wrks
Copy link
Author

Hey thanks for the answer regarding my question!

Great to see the boolean wrapped inside the usual object, so it's consistent with other values.
Your screenshot shows "value" as a JS boolean. I wonder if that's the best choice. Here are my thoughts:

In the IFC raw text, boolean values look liky any other enumeration. For example here:

Image

.NOTDEFINED. is part of the "IfcWindowStyleConstructionEnum" in this example:

Image

.F. represents an IfcBoolean, which may be seen as part of an enumeration definition F and T.
However, there's also IfcLogical, which can be .F., .T. or .U. in the IFC raw text. For the third state UNKNOWN, there's no JS equivalent.
Since all enumerations usually resolve to strings, it would be consistent to simply return value: "F"/"T"/"U".
So type: 3 would always represent a string and there would be no problem with translating the IfcLogical .U. state.

I hope this makes sense, I guess we all wish IFC would just be less complex... :-)

@andy-wrks
Copy link
Author

I have just seen at the very bottom of your pull request that UNKNOWN will be translated to JS primitive value 'undefined'. So that case is actually handled, which I wasn't sure of in my last comment.
Although a bit unexpected, I can live with the fact that type: 3 can now either be a JS string, boolean or undefined.

Thanks for fixing this issue and properly wrapping these values in the usual object! I'm looking forward to the next release!

@liradb2000
Copy link
Contributor

Thanks for sharing your idea.

I initially thought that the type property was simply an indicator of how to process reading or writing values in an IFC file.
So, using JavaScript primitive types directly seems less confusing than using an enum, I think—especially for JavaScript users who cannot use enums.

However, TypeScript users would need to define a variable that allows string, boolean, and undefined to use the value, which could further increase confusion.

In my opinion,
type = 3 represents an ENUM, and it is not just a primitive string type.
It can literally ENUM represent various primitive types (Heterogeneous enums).
However, defining an enum and returning or using it as the value would make things much clearer.

For example,

const IFCLOGIC_ENUM= {
  TRUE : true,
  FALSE : false,
  UNKNOWN : undefined,
} as const;

type IFCLOGIC_VALUE = typeof IFCLOGIC_ENUM[keyof typeof IFCLOGIC_ENUM];

@beachtom
Copy link
Collaborator

Yes I like this approach - thanks for your contributions here.

@beachtom
Copy link
Collaborator

@andy-wrks do you think this can be considered closed with the PR now merged?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants