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

SOLID Principles refactoring. #1

Open
lolothens-e opened this issue Nov 9, 2024 · 0 comments
Open

SOLID Principles refactoring. #1

lolothens-e opened this issue Nov 9, 2024 · 0 comments

Comments

@lolothens-e
Copy link

SOLID Principles Violations

Hello there, as a part of a uni course I've identified at least 7 violations regarding SOLID principles in your code. I understand these are learning tools and therefore don't have to follow good design for the sake of learning and you can dismiss this if you want, however I'll explain such violations.

  1. SRP: Single Responsibility

- Switch use with methods implementations in the the 3 projects main classes, specifically in the communications project.
image

Your menus are all within a main method in your class, this makes it look cluttered and also makes it very hard to understand even with comments. I've attached UML diagrams to illustrate my point. This addresses some of the problems making it look a tad better.

However the code still violates the principle at the class level so this is a revised version of the UML.

image

The code with some suggested changes (altough non-functional) can be found here: Comms suggested changes

  1. OCP: Open-Closed

There's 2 major fixes I'd suggest making here, the first one has once again something to do with the use of the switch as your main way to implement options in a menu since for every new feature added you'd need to get to the heart of your code. In order to unclutter this I've made this UML diagram.

image

Suggesting use of interfaces, classes for options and just method calls for code execution.

Altough most implemented, the code can be seen and reviewed here: Maket suggested changes

The second major pain point comes from the ship creation logic, and this is what-if kind of situation given that the code works and run very well with the testfiles. My suggestion for that would be dettaching a bit of the responsibility into some other classes so in case ship creation logic changes you can easily expand on mentioned class and not on main directly. UML diagram and the attached code will be below.

image

And the code can be found here: Ship suggested changes

  1. LSP: Liskov Substitution Principle
    Lastly, on the marketplace projects there's an opportunity to avoid LSP violation regarding the Order, BuyingOrder and SellingOrder.
    You can swap the Order object for an interface and have the Buying and Selling kind be implementations of this one,

You can find UML diagrams and code here:

image

And the code: Maket suggested changes

Thanks for your work in this repo and I hope this isn't bothersome.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant