Added "Setup" methods to do the Actions input setup that was in BHoM_UI

BHoM / BHoM_Adapter

GNU Lesser General Public License v3.0

7 stars 5 forks source link

Added "Setup" methods to do the Actions input setup that was in BHoM_UI #264

Closed alelom closed 3 years ago

alelom commented 3 years ago

NOTE: Depends on

https://github.com/BHoM/BHoM_UI/pull/329

Issues addressed by this PR

Closes https://github.com/BHoM/BHoM_Adapter/issues/259

Moves the logic related to the Adapter Actions setup from the BHoM_UI in the Adapter.
Advantages:

Flexibility: Toolkits can now override the new SetupThen[ActionName] methods if they need to have different set-up.
Consistency: logic related to the adapter is all in one place. See https://github.com/BHoM/BHoM_Adapter/issues/259 for more info.
Does not require any alignment: all Toolkits will work as before.

I need this quite urgently now because of some needed changes in other toolkits (File_Adapter).

Test files

Changelog

Additional comments

alelom commented 3 years ago

Most of the code in here is aimed at handling null cases. So wondering if it wouldn't make more sense to have that directly into to core method instead of a separate one. Genuine question though as I haven't been involved with the adapters long enough that my question might have an obvious answer.

The main point of having the logic here is to allow overriding by Toolkits. This gives the flexibility of adding checking/verification/additional settings to the "SetUp" methods, or replacing entirely what we do at the base. This is useful in several scenarios.

IsakNaslundBh commented 3 years ago

Most of the code in here is aimed at handling null cases. So wondering if it wouldn't make more sense to have that directly into to core method instead of a separate one. Genuine question though as I haven't been involved with the adapters long enough that my question might have an obvious answer.

To expand a bit on alessios answer, it is more about defaults and converts than null checks, and making it possible for each adapter to handle that differently. For example, being able to convert a string, formatted in a specific way into a request that is friendly to that particular adapter, but non-sensical outside that scope.

As for the ActionConfigs, could be used by RevitToolkit for example, that make use of that mechanism a lot, to directly instantiate the appropriate ActionConfig required by a particular action, such as Pull.

alelom commented 3 years ago

/azp run BHoM_Adapter.CheckInstaller

azure-pipelines[bot] commented 3 years ago

Azure Pipelines successfully started running 1 pipeline(s).