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

rename and make properties accessible #1063

Merged
merged 4 commits into from
Nov 28, 2023
Merged

Conversation

anthonie-kramer
Copy link
Contributor

@anthonie-kramer anthonie-kramer commented Nov 23, 2023

BACKGROUND:

  • Doors needed some properties exposed for functions... This way doors can be adapted depending on context (e.g. wall thickness)

DESCRIPTION:

  • Makes properties public so users can define these in their function as needed

TESTING:

  • These are being used in the updated Doors and Interior Partitions functions

FUTURE WORK:

  • Possibly more refactoring as we make this cleaner

REQUIRED:

  • All changes are up to date in CHANGELOG.md.

COMMENTS:

  • Any other notes.

This change is Reviewable

Copy link
Member

@wynged wynged left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed 6 of 6 files at r1, all commit messages.
Reviewable status: 1 change requests, 0 of 1 approvals obtained (waiting on @anthonie-kramer)


Elements/test/DoorTest.cs line 17 at r1 (raw file):

            var line = new Line(new Vector3(0, 0, 0), new Vector3(10, 10, 0));
            var wall = new StandardWall(line, 0.1, 3.0);
            var door = new Door(wall.CenterLine, 0.5, 2.0, 2.0, 2 * 0.0254, DoorOpeningSide.LeftHand, DoorOpeningType.SingleSwing);

this value probably wants to be a const like DEFAULT_DOOR_THICKNESS


Elements.Serialization.IFC/src/IFCToHypar/Converters/FromIfcDoorConverter.cs line 40 at r1 (raw file):

            var doorWidth = (IfcLengthMeasure)ifcDoor.OverallWidth;
            var doorHeight = (IfcLengthMeasure)ifcDoor.OverallHeight;
            var doorThickness = 2 * 0.0254;

use same const as above.


Elements.Serialization.IFC/test/IFCTests.cs line 141 at r1 (raw file):

            var wall2 = new StandardWall(wallLine2, 0.2, 2, name: "Wall2");

            var door1 = new Door(wallLine1, 0.5, 1.5, 2.0, 2 * 0.0254, DoorOpeningSide.LeftHand, DoorOpeningType.DoubleSwing);

maybe also use the const? less important in test.

Copy link
Contributor Author

@anthonie-kramer anthonie-kramer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewable status: 1 change requests, 0 of 1 approvals obtained (waiting on @wynged)


Elements/test/DoorTest.cs line 17 at r1 (raw file):

Previously, wynged (Eric Wassail) wrote…

this value probably wants to be a const like DEFAULT_DOOR_THICKNESS

Done.


Elements.Serialization.IFC/src/IFCToHypar/Converters/FromIfcDoorConverter.cs line 40 at r1 (raw file):

Previously, wynged (Eric Wassail) wrote…

use same const as above.

Done.


Elements.Serialization.IFC/test/IFCTests.cs line 141 at r1 (raw file):

Previously, wynged (Eric Wassail) wrote…

maybe also use the const? less important in test.

Done.

Copy link
Member

@wynged wynged left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed 4 of 4 files at r2, all commit messages.
Reviewable status: :shipit: complete! 1 of 1 approvals obtained

@anthonie-kramer anthonie-kramer merged commit b905514 into master Nov 28, 2023
1 check passed
@anthonie-kramer anthonie-kramer deleted the door-updates-again branch November 28, 2023 22:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants