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

Common_Engine: IElementCurves uses BHoMObject #1323

Closed
kThorsager opened this issue Nov 13, 2019 · 5 comments · Fixed by #1498
Closed

Common_Engine: IElementCurves uses BHoMObject #1323

kThorsager opened this issue Nov 13, 2019 · 5 comments · Fixed by #1498
Assignees
Labels
size:XS Measured in seconds type:bug Error or unexpected behaviour type:compliance Non-conforming to code guidelines

Comments

@kThorsager
Copy link
Contributor

kThorsager commented Nov 13, 2019

Description:

image
IElement is not a BHoMObject. This still "works" as it's mostly used for Tex Bar & Panel which also is a BHoMObject.

The only thing this does is sending it to a decrepit method which then casts it to IElement to then call the new one. Hence simply changing BHoMObject to IElement would fix this.

foreach (BHoMObject element in elements)

@kThorsager kThorsager added size:XS Measured in seconds type:bug Error or unexpected behaviour type:compliance Non-conforming to code guidelines labels Nov 13, 2019
@FraserGreenroyd
Copy link
Contributor

I would agree that it would make logical sense when reading the code for the for loop to be
foreach(IElement element in elements)
than what is currently there. I see no reason why the for cannot be updated per the suggestion @kThorsager is making?

@IsakNaslundBh and @pawelbaran if you're in agreement I propose @kThorsager make the change for PR and review? 😄

@FraserGreenroyd FraserGreenroyd removed their assignment Nov 13, 2019
@pawelbaran
Copy link
Member

Yeah, this is legacy from the times when there was no IElement interface - should have been changed when the latter was being implemented. I believe the issue should be resolved while migrating all IElement-related stuff to Geometry_Engine.

@kThorsager
Copy link
Contributor Author

Ok, then I'll add this in the TODO of #1316 whenever the Geometry_Engine .csproj is free

@pawelbaran
Copy link
Member

@kThorsager is this still in progress or already resolved?

@kThorsager
Copy link
Contributor Author

In progress in #1498
Was a bit distracted from all the planning around Geometry vs Common vs Spatial Engine

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
size:XS Measured in seconds type:bug Error or unexpected behaviour type:compliance Non-conforming to code guidelines
Projects
None yet
4 participants