You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
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.
SRP: Single Responsibility
- Switch use with methods implementations in the the 3 projects main classes, specifically in the communications project.
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.
The code with some suggested changes (altough non-functional) can be found here: Comms suggested changes
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.
Suggesting use of interfaces, classes for options and just method calls for code execution.
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.
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,
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.
- Switch use with methods implementations in the the 3 projects main classes, specifically in the communications project.
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.
The code with some suggested changes (altough non-functional) can be found here: Comms suggested changes
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.
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.
And the code can be found here: Ship suggested changes
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:
And the code: Maket suggested changes
Thanks for your work in this repo and I hope this isn't bothersome.
The text was updated successfully, but these errors were encountered: