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

Refactoring and documentation based on "Universal" common interfaces #17

Open
19 tasks
alelom opened this issue Jul 26, 2021 · 3 comments
Open
19 tasks
Assignees
Labels
type:compliance Non-conforming to code guidelines

Comments

@alelom
Copy link
Member

alelom commented Jul 26, 2021

Broken rules:

There are some keys interfaces that drive everything, defining the formal structure, dependency and organization of the BHoM.

We need to push a reorganization of their arrangement, tweak some conventions and namings, and relocate appropriately certain Engine methods that use/depend on them.

Suggestions to restore compliance:

WIP diagram:
image

Engines

BHoM_Engine

  • Move the IGeometry and IGeometry3D methods into the Geometry_Engine.

Analytical_Engine-Graphics_Engine

  • Methods that generalize the IResult representation.
  • [ ]

Results_Engine

The Results engine should be deleted, as it currently depends on Structure_oM and Analytical_oM and Graphic_oM and its categorization is not meaningful.

  • Move the Max, Min, Total, AbsoluteMax methods into Structure.Query.
  • Move the DisplayMeshResults into Graphic_oM. Add Analytical_oM dependency in Graphic_oM.
  • Move Analytical_Engine: Group, IsNull, MapResults (to be renamedMappedResult).
  • Delete the Result_Engine.

Reflection_Engine

Any objects/methods left obviously is code that is needed as part of the Framework for Adapters, UIs etc. and is distinct from the already existing Programming namespace that is "Programming as a discipline". i.e. creating Programming representation of a concept much like a Structural or Acoustics representation say

Object models

Base_oM

  • Add description to the BHoMGroup and IBHoMGroup.
  • Move the ISpecification interface in the CIH_Toolkit.
  • Review the ComparisonConfig and if it makes sense to move it in the Diffing_oM.
  • Review the usage of ISettings interface - if not used, remove; or see if it can be moved below.

Analytical_oM

Graphical_oM

Reflection_oM / Programming_oM / Data_oM

EDIT: Updated the diagram to latest WIP and point around Reflection namespace in light of updated diagram

@FraserGreenroyd
Copy link
Contributor

Both BHoM/BHoM_Engine#1645 and BHoM/BHoM_Engine#2671 are related and focus on the same issue of migrating stuff from Reflection to Base as appropriate.

I am intending to close these out in sprint 1 of the 5.1 milestone in the new year. The following steps will be undertaken:

  • Move Attributes from Reflection_oM to BHoM - these are fundamental to the describing of the BHoM and should be easily accessible to all projects from Base rather than an additional dependency
  • Move Record_*, BHoMTypeList ExtensionMethodToCall and RunExtensionMethod methods from Reflection_Engine to BHoM_Engine - these are used by Base and are crucial to the running of the BHoM, so again removing dependency on Reflection_Engine unless a project is actually requiring some form of reflection
  • These will be PRed together on one branch name across oM and Engine
  • BHoMBot will then comb through all other installer repositories and make as many automatic updates as possible to the references for calling the above mentioned methods and attributes - some manual assistance may be required but check core and installer will assist with picking these up
  • This will then be reviewed and merged as appropriate (pending zero-code inclusions, see below)
  • A wiki page will be established outlining these changes for other repos which are not in the installers to aid people in making updates as necessary
  • BHoMBots functionality for auto-updating code will be made available for those wishing to use it on non-installer repos.

There are some non-installer repos which should be updated at the same time, namely around the zero-code tools. @adecler could you add a list of the repos you'd like to see updated at the same time for continuity to one of the linked issues (to avoid adding too much more to this issue than is necessary for this issue) (or as an edit to this comment for me)?

Hopefully that all makes sense. As I say, timeline is sprint 1 of 5.1 milestone, probably towards the end of the first week owing to the copyright updates which will happen first and would cause conflicts.

@al-fisher
Copy link
Member

Thanks @FraserGreenroyd @adecler
Plan sounding good.

N.B. I have updated the diagram in this issue above to latest iteration.
Useful to ensure mapping all methods being migrated (as well as all methods to remain in Reflection Engine) to the classifications of this diagram. So we are all happy makes sense and is logical going forward.

Equally see updated comment above about any code remaining in current Reflection oM/Engine after migration. What will be left and should we rename the Engine to help clarify its purpose? Ideas welcome.

@IsakNaslundBh
Copy link

IsakNaslundBh commented Dec 23, 2021

Agree with 95% of all the suggestions for changes in this issue. Some few things I slightly question are:

Move the IGeometry and IGeometry3D methods into the Geometry_Engine.

Personally, I want to get rid of any reference to any BHoMObject from the Geometry_Engine, and for it to only work with pure geometric concepts. This goes in line with the not yet fully closed out BHoM/BHoM#756 issue (which probably could/should be added to this list to finally get levels and grids moved out of Geometry_oM).

Can understand the reasoning behind getting it out of the Base_oM, even if I kind of like to have it there myself, but if it needs to be moved I would argue a better fit for it would be the Spatial_Engine, for the same reasons as discussed in the linked BHoM issue.

Review the usage of ISettings interface - if not used, remove; or see if it can be moved below.

This is being used be analytics and Library, and more to come, and wonder if it is not best placed in the base oM? I personally at least have a hard time coming up with a better place for it. Know the whole statement starts with review, so might be the conclusion of that review. 😄

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type:compliance Non-conforming to code guidelines
Projects
None yet
Development

No branches or pull requests

4 participants