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

Refactor execution logic from CLI into command design pattern #51

Closed
1 task done
rszamszur opened this issue Feb 13, 2022 · 1 comment · Fixed by #52
Closed
1 task done

Refactor execution logic from CLI into command design pattern #51

rszamszur opened this issue Feb 13, 2022 · 1 comment · Fixed by #52
Assignees
Labels
refactor Related to major code refactoring/reorganisation
Milestone

Comments

@rszamszur
Copy link
Member

  • I have searched the issues of this repo and believe that this is not a duplicate.

Feature Request

Refactor commands execution logic into the strategy design pattern.

There already was one ticket for refactoring logic from CLI: #38. However, it mainly refactored common code parts into separate classes for reusability across the entire project. But, there still are some logic leftovers inside CLI commands. Each feature/enhancement/bug_fixe might grow it, which makes testing more complex, code less readable, and more difficult to extend and maintain codebase. This is already taking its toll at the current stage of development.

Benefits:

  • Possibility to swap algorithms used inside an object at runtime
  • Isolate the implementation details of an algorithm from the code that uses it.
  • Code more oriented towards single-responsibility principle and open–closed principle
  • Ease of adding a new interface or implementing API for programmatically executing fastapi-mvc actions. (Not that this is utterly needed, but still a benefit)
  • Should be easier to unit test and maintain the codebase

Tradeoffs:

  • Refactoring effort, and the possibility of bugs introduced with this change.
  • One needs to know the strategy design pattern. This can make entry-level more complex.
  • This approach assumes one can wrap algorithms implementation in some kind of abstraction. The less similarity between implementations (method signature, needed classes/functions, etc.), the more difficult it is to implement and utilize this pattern.
@rszamszur rszamszur added the refactor Related to major code refactoring/reorganisation label Feb 13, 2022
@rszamszur rszamszur self-assigned this Feb 13, 2022
@rszamszur rszamszur added this to the 0.9.0 milestone Feb 13, 2022
@rszamszur rszamszur mentioned this issue Feb 13, 2022
4 tasks
@rszamszur rszamszur changed the title Refactor commands execution logic into strategy design pattern Refactor commands execution logic into command design pattern Feb 14, 2022
@rszamszur
Copy link
Member Author

On second thought, command design pattern will be a much better fit.

Since strategy and command are very similar, the benefits and tradeoffs are nearly the same. The most significant swap here is:
Possibility to swap algorithms used inside an object at runtime to the possibility to implement undo/redo

which is IMHO much better scenario for this project. Ex, in the future generators, can have --dry-run options, or possibility to "ungenerate" ex: fastapi-mvc generate ... fastapi-mvc delete ...

Moreover, the codebase will be easier to understand and follow. Last but not least, a strategy design pattern is a good fit when you have a lot of similar classes that only differ in the way they execute some behavior. Which isn't the case here. Different commands can have totally different execution methods signatures and may require some before and after logic.

In the end, refactoring effort from strategy to command is very low because of their similarity.

@rszamszur rszamszur changed the title Refactor commands execution logic into command design pattern Refactor execution logic from CLI into command design pattern Feb 14, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
refactor Related to major code refactoring/reorganisation
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant