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

Improve the Id query #288

Closed
pawelbaran opened this issue Feb 17, 2021 · 6 comments · Fixed by #290
Closed

Improve the Id query #288

pawelbaran opened this issue Feb 17, 2021 · 6 comments · Fixed by #290
Assignees
Labels
type:feature New capability or enhancement type:question Ask for further details or start conversation

Comments

@pawelbaran
Copy link
Member

pawelbaran commented Feb 17, 2021

Description:

During today's workshop with the new users I got mindblown by how hard it is to extract the Id of an object in a given software. The workshop participants received a double blow when they needed to quickly learn what needs to be plugged in to adapterIdFragmentType, what a type is and what do fragments have to do with it 😃

IMO the UX of this is unacceptable, I did not even know that it works this way as I did create an ElementId query method in Revit_Toolkit a long time ago. Couldn't we simply have a set of methods like RobotId, EtabsId, NameYourFavSoftwareId? This will make it SO much easier for the users, who may not know what adapter is, but will for sure start the search of e.g. Robot Id from typing 'robot id' in the search bar.

image

@al-fisher
Copy link
Member

❤️ 📎

@alelom
Copy link
Member

alelom commented Feb 18, 2021

Thanks @pawelbaran, this is something I've always thought too.

I welcome you to review #287 where this is now improved as much as physically feasible from the BHoM_Adapter alone. In that PR, I'm giving more clarity to the methods available to get the AdapterId.

  • AdapterId(IBHoMObject bHoMObject, Type adapterIdFragmentType) (singular) is a method that returns 1 adapter Id only of the provided fragmentType. Since multiple Ids of different adapters may be stored on an object, this method asks that a FragmentType of the required Id is input.

  • AdapterIds(IBHoMObject bHoMObject) (plural) is a method that returns all adapterIds stored on the object. If you specify an optional Type adapterIdFragmentType input, then only the Ids of that type are retrieved.

This is the best we can do on the BHoM_Adapter, which has to abstract away from the AdapterIds implementations downstream.

As you say, however, we can do even better:

Couldn't we simply have a set of methods like RobotId, EtabsId, NameYourFavSoftwareId?

Absolutely! This is a very good idea, but naturally it's not implementable in the BHoM_Adapter, because obviously we can't reference the oMs of downstream Toolkits.

Suggestion: suggest as an action on every Toolkit that implements AdapterId to implement also a Query method (e.g. RevitID(BHoMObject obj), RobotID(BHoMObject obj) etc.) that returns that Id for any BHoMObject.

@alelom alelom added the type:question Ask for further details or start conversation label Feb 18, 2021
@pawelbaran
Copy link
Member Author

Thanks @alelom for the reply, I sort of deducted all that when thinking why does it work this way. Great to hear that you agree with my idea of adding extra Query methods - I think it will be a cheap and robust solution that will remove 99% of friction on the user side.

By the way, this issue has been raised after working on #287 branch, so the problem will still be relevant after the improvements.

@alelom
Copy link
Member

alelom commented Feb 18, 2021

By the way, this issue has been raised after working on #287 branch, so the problem will still be relevant after the improvements.

Because it's raised under BHoM_Adapter, #287 fixes this issue as much as physically feasible from the BHoM_Adapter perspective.

There's another place for the distributed issues across multiple repos, could your raise it there? https://github.com/BHoM/admin/issues

@pawelbaran
Copy link
Member Author

I am closing this issue as the discussion is moving to BHoM/admin#15.

@alelom
Copy link
Member

alelom commented Feb 23, 2021

@pawelbaran thanks, I actually would like to let #287 close this Issue, as it actually applies some simplification on the Adapter side.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type:feature New capability or enhancement type:question Ask for further details or start conversation
Projects
None yet
5 participants