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

File_Adapter: refactoring-refactor to match ReplaceInMemory refactoring #149

Open
alelom opened this issue Jul 2, 2019 · 0 comments
Open
Assignees
Labels
type:compliance Non-conforming to code guidelines

Comments

@alelom
Copy link
Member

alelom commented Jul 2, 2019

Description

As explained in BHoM/BHoM_Adapter#106

Since File_Adapter does not set AdapterConfig.ProcessInMemory, that defaults to true.
This means that the call to Create will always have replaceAll set to true.

Therefore the following calls will have replaceAll always set to true:
https://github.com/BHoM/BHoM_Adapter/blob/49d63e82db3eefa5eecb960e1bd84ab410fc2c51/File_Adapter/CRUD/Create.cs#L40-L46

So these lines never get called
https://github.com/BHoM/BHoM_Adapter/blob/49d63e82db3eefa5eecb960e1bd84ab410fc2c51/File_Adapter/CRUD/Create.cs#L80-L81
As well as this is never called with Append:
https://github.com/BHoM/BHoM_Adapter/blob/49d63e82db3eefa5eecb960e1bd84ab410fc2c51/File_Adapter/CRUD/Create.cs#L57

Proposed change

If anything, the option for the Create method to override should be specifiable dynamically, rather than hard-coded.
This might be solved by making the so called config parameter available to all CRUD methods.

I would remove replaceAll as a parameter of the Create() and use another parameter like
config["CreateOverride"] = true.

@alelom alelom self-assigned this Jul 2, 2019
@alelom alelom added the type:compliance Non-conforming to code guidelines label Jul 2, 2019
@alelom alelom changed the title File_Adapter: refactor to match ReplaceInMemory refactoring File_Adapter: refactoring-refactor to match ReplaceInMemory refactoring Jul 3, 2019
This issue is being transferred. Timeline may not be complete until it finishes.
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

1 participant