I have attempted to improve the following things in this commit. Please take a look and share your opinion.
Ensure that all public structures are in the top part of the file and all private functions and structs are in the bottom part of the file.
Add documentation that is reasonable for now, to all the public contracts
Simplify the public contract as much as possible. So in this case, we have a Workflow, PGWorkflow as types, and CreateWorkflow as the method to create a workflow and ToWorkflow and ToPGWorkflow as the utility conversion methods. To the outside world, this makes the contract small and understandable.
Instead of doing an if/else or switch case, the class has an enricherMap which holds a step type and an anonymous function for it that will be applied.
Also note that key is not a plain string, but an alias type for a string called stepType. This adds to readability. Since we want to scope visibility, the type is private.
The value in the map is also an alias type for a function that takes a step, id for step and returns back an updated step. This allows multiple-step types to be added. However, overall we can possibly find a better mechanism to enrich steps and sub-steps.
I think from the code it is evident that sub-steps were added later on as we handle the id assignment for them separately instead of something like recursion. But I'll think and improve this a bit more.
I have not reviewed the tests and will do that next to improve the coverage and style. Please take a look and let's have a discussion. Having the discussion will promote consistency in code as we all will agree on things.
I have attempted to improve the following things in this commit. Please take a look and share your opinion.
Workflow
,PGWorkflow
as types, andCreateWorkflow
as the method to create a workflow andToWorkflow
andToPGWorkflow
as the utility conversion methods. To the outside world, this makes the contract small and understandable.stepType
. This adds to readability. Since we want to scope visibility, the type is private.I think from the code it is evident that sub-steps were added later on as we handle the id assignment for them separately instead of something like recursion. But I'll think and improve this a bit more.
I have not reviewed the tests and will do that next to improve the coverage and style. Please take a look and let's have a discussion. Having the discussion will promote consistency in code as we all will agree on things.