Refactor and add ConversionConfig class as argument to createDcat #24
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Refactor and add ConversionConfig class as argument to createDcat
Description
This pull request addresses issue #22 and makes it possible to specify a working directory where files are produced when calling the method
Manager.createDcat
. As part of that, I made the following changes:ConversionConfig
that contains (1) theapiSpecMap
and (2) theworkDir
working directory. This class wraps the data consumed byManager.createDcat
.Manager.createFromFile
andManager.createFromDirectory
to this newConversionConfig
class as methodsfromDirectory
andfromFile
.Manager.createDcat
implementation take aConversionConfig
instead of aMultiValueMap
.ConversionConfig
.Manager.createFromList
method where the exception message will be pushed twice toresults
ifcreateDcat
throws an exception. This bug was fixed as a consequence of the refactoring.Fixes #22
Discussion
The motivation for this PR was that I wanted to add a way of specifying the path where files are produced, as a
workDir
parameter. For that, I added theConversionConfig
class that now groups theapiSpecMap
andworkDir
parameter. However, theworkDir
parameter could also be part of theManager
. I am unsure about what is best. If we would moveworkDir
from theConversionConfig
class toManager
, then theConversionConfig
class would only contain one instance variable,apiSpecMap
, and wouldn't be necessary anymore. At the same time, I am not sure it is a bad idea to have aConversionConfig
class that wraps theapiSpecMap
parameter, because it makes clarifies intent and makes the code more easily extendable to adding more parameters.Checklist