Open ghost opened 3 years ago
OPEN CLOSED VIOLATION
In addition to this comment, I would like to comment that I consider that the open closed principle is also being violated. This consideration is due to the fact that the Liskov principle and that of OCP are closely linked (Mohamed sanaulla (2011)). This is because, if the “child” class in this case "LinkSelector" cannot be referenced by the “Parent” class BaseElementSelector, it will tend to make modifications for this to happen and not only that, different selectors can also be added. that will grow the functionalities for which the code was initially created. With this, it can be clearly seen how the principle is being violated, since not only new classes or instances are created, but large code modifications occur that do not allow it to be closed for modifications.
One of the solutions that will allow your code to follow the aforementioned design principles would be the use of interfaces (Just as the participant mentions in the main comment), since, if it is required to expand any of the classes, the program will remain closed for modifications.
Thank you in advance for reading this comment, as I mentioned this is a consideration and could be wrong, since I am a student who is just putting into practice her knowledge about the SOLID principles
Reason.
There are multiple classes inside Selector package on https://github.com/code4craft/webmagic/tree/master/webmagic-core that extends or implements from a superType or a interface and in some cases this classes dont implement all methods from their inheritance or interface. Throwing a UnsupportedOperationException() (I think the proper word is dummy method?) in these implementations is indeed a violation accordingly to the concepts of LSP.
"No new exceptions should be thrown by methods of the subtype, except where those exceptions are themselves subtypes of exceptions thrown by the methods of the supertype".
Solution.
As a solution to these i recommend a better abstraction of these classes, separating them properly or using another interfaces which will look kinda messy in your code but will solve future additions to your code. Here some examples of solution i could think of:
UML Diagrams and portion of codes.
Illustration 1.1 - Violation UML superType interface. Illustration 1.2 - Possible Solution to LSP UML Diagram. Illustration 1.3 - Possible solution to LSP UML Diagram. Illustration 1.4 - Possible solution to LSP in the interface. Illustration 1.5 - ReplaceSelector in which i deleted the function that wasnt being implemented correctly and instead of using Selector, i used an isolated interface SelectText which implements the selectText() method that needs. Illustration 1.6 - LinkSelector in which i deleted methods that throws an exception over an override method, as a solution i think that LinkSelector should not extend from BaseElementSelector just to make it more LSP functional.
References i used.
If there is some kind of error i would love to have your feedback because im still learning Design Principles and Clean Code so any comment from someone quite experienced like you will help me in growing my knowledge and experience. Thank you for the attention. Keep it up!