A function should do one, and only one, thing. We use that principle when we are refactoring large functions into smaller functions; we use it at the lowest levels
A module should have one, and only one, reason to change.
A module should be responsible to one, and only one, actor.
A software artifact should be open for extension but closed for modification.
THE SQUARE/RECTANGLE PROBLEM
ISP: THE INTERFACE SEGREGATION PRINCIPLE
it is harmful to depend on modules that contain more than you need.
DIP: THE DEPENDENCY INVERSION PRINCIPLE
Source code dependencies refer only to abstractions, not to concretions.
interfaces are less volatile than implementations.
Which classes belong in which components?
THE REUSE/RELEASE EQUIVALENCE PRINCIPLE
THE COMMON CLOSURE PRINCIPLE
Gather into components those classes that change for the same reasons and at the same times. Separate into different components those classes that change at different times and for different reasons.
THE COMMON REUSE PRINCIPLE
Don’t force users of a component to depend on things they don’t need.
Orthogonality
Two or more things are orthogonal if
changes in one do not affect any of the others. In a well-designed system, the database code
will be orthogonal to the user interface: you can change the interface without affecting the
database, and swap databases without changing the interface.
Every Piece Of Data Should Have One Representation
Functions Should Do One Thing
Use Dummy Data
If It’s Taking Too Long You Are Probably Making a Mistake
Do Things The Best Way The First Time
Follow Conventions
PyCharm
"Good design adds value faster than it adds cost."
The Four Pillars of Object-oriented
inheritance, polymorphism, abstraction and encapsulation
SRP AND THE DECORATOR PATTERN
The Composite pattern is a specialization of the Decorator pattern
The Composite pattern’s purpose is to allow you to treat many instances of an interface as if they were just one instance. Therefore, clients can accept just one instance of an interface, but they can be implicitly provided with many instances, without requiring the client to change
Predicate decorators
Branching decorators
The open/closed principle
Software entities should be open for extension, but closed for modification.
A more permissive exception to the “closed for modification” rule is that any change to existing code is allowed as long as it does not also require a change to any client of that code.
Classes that honor the OCP should be open to extension by containing defined extension points where future functionality can hook into the existing code and provide new behaviors.
“Design for inheritance or prohibit it”
Identify points of predicted variation and create a stable interface around them.
A stable interface
Just enough adaptability
The predicted variation guideline states that you should be explicit in what you allow and disallow to be extended. The speculative generality guideline states that you should beware of trying to preempt how a class might apply to a general problem, lest you create a leaky abstraction.
http://blog.jetbrains.com/upsource/2015/08/31/what-to-look-for-in-a-code-review-solid-principles-2/
A module should have one, and only one, reason to change.
A module should be responsible to one, and only one, actor.
A software artifact should be open for extension but closed for modification.
THE SQUARE/RECTANGLE PROBLEM
ISP: THE INTERFACE SEGREGATION PRINCIPLE
it is harmful to depend on modules that contain more than you need.
DIP: THE DEPENDENCY INVERSION PRINCIPLE
Source code dependencies refer only to abstractions, not to concretions.
interfaces are less volatile than implementations.
Which classes belong in which components?
THE REUSE/RELEASE EQUIVALENCE PRINCIPLE
THE COMMON CLOSURE PRINCIPLE
Gather into components those classes that change for the same reasons and at the same times. Separate into different components those classes that change at different times and for different reasons.
THE COMMON REUSE PRINCIPLE
Don’t force users of a component to depend on things they don’t need.
Orthogonality
Two or more things are orthogonal if
changes in one do not affect any of the others. In a well-designed system, the database code
will be orthogonal to the user interface: you can change the interface without affecting the
database, and swap databases without changing the interface.
Every Piece Of Data Should Have One Representation
Functions Should Do One Thing
Use Dummy Data
If It’s Taking Too Long You Are Probably Making a Mistake
Do Things The Best Way The First Time
Follow Conventions
PyCharm
"Good design adds value faster than it adds cost."
The Four Pillars of Object-oriented
inheritance, polymorphism, abstraction and encapsulation
SRP AND THE DECORATOR PATTERN
The Composite pattern is a specialization of the Decorator pattern
The Composite pattern’s purpose is to allow you to treat many instances of an interface as if they were just one instance. Therefore, clients can accept just one instance of an interface, but they can be implicitly provided with many instances, without requiring the client to change
Predicate decorators
Branching decorators
The open/closed principle
Software entities should be open for extension, but closed for modification.
A more permissive exception to the “closed for modification” rule is that any change to existing code is allowed as long as it does not also require a change to any client of that code.
Classes that honor the OCP should be open to extension by containing defined extension points where future functionality can hook into the existing code and provide new behaviors.
“Design for inheritance or prohibit it”
Identify points of predicted variation and create a stable interface around them.
A stable interface
Just enough adaptability
The predicted variation guideline states that you should be explicit in what you allow and disallow to be extended. The speculative generality guideline states that you should beware of trying to preempt how a class might apply to a general problem, lest you create a leaky abstraction.
http://blog.jetbrains.com/upsource/2015/08/31/what-to-look-for-in-a-code-review-solid-principles-2/
- S – Single Responsibility Principle
- O – Open-Closed Principle
- L – Liskov Substitution Principle
- I – Interface Segregation Principle
- D – Dependency Inversion Principle
Single Responsibility Principle (SRP)
There should never be more than one reason for a class to change.
This can sometimes be hard to spot from a single code review. By definition, the author is (or should be) applying a single reason to change the code base – a bug fix, a new feature, a focussed refactoring.
You want to look at which methods in a class are likely to change at the same time, and which clusters of methods are unlikely to ever be changed by a change to the other methods. For example:
This side-by-side diff from Upsource shows that a new piece of functionality has been added to
TweetMonitor
, the ability to draw the top ten Tweeters in a leaderboard on some sort of user interface. While this seems reasonable because it uses the data being gathered by theonMessage
method, there are indications that this violates SRP. The onMessage
and getTweetMessageFromFullTweet
methods are both about receiving and parsing a Twitter message, whereas draw
is all about reorganising that data for displaying on a UI.
The reviewer should flag these two responsibilities, and then work out with the author a better way of separating these features: perhaps by moving the Twitter string parsing into a different class; or by creating a different class that’s responsible for rendering the leaderboard.
Open-Closed Principle (OCP)
Software entities should be open for extension, but closed for modification.
As a reviewer, you might see indications that this principle is being violated if you see a series of
if
statements checking for things of a particular type:
If you were reviewing the code above, it should be clear to you that when a new
Event
type is added into the system, the creator of the new event type is probably going to have to add another else
to this method to deal with the new event type.
It would be better to use polymorphism to remove this
if
:
As always, there’s more than one solution to this problem, but the key will be removing the complex
if/else
and the instanceof
checks.
Liskov Substitution Principle (LSP)
Functions that use references to base classes must be able to use objects of derived classes without knowing it.
One easy way to spot violations of this principle is to look for explicit casting. If you have to cast a object to some type, you are not using the base class without knowledge of the derived classes.
More subtle violations can be found when checking:
Imagine, for example, we have an abstract
Order
with a number of subclasses – BookOrder
,ElectronicsOrder
and so on. The placeOrder
method could take a Warehouse
, and could use this to change the inventory levels of the physical items in the warehouse:
Now imagine we introduce the idea of electronic gift cards, which simply add balance to a wallet but do not require physical inventory. If implemented as a
GiftCardOrder
, theplaceOrder
method would not have to use the warehouse parameter:
This might seem like a logical use of inheritance, but in fact you could argue that code that uses
GiftCardOrder
could expect similar behaviour from this class as the other classes, i.e. you could expect this to pass for all subtypes:
But this will not pass, as
GiftCardOrders
have a different type of order behaviour. If you’re reviewing this sort of code, question the use of inheritance here – maybe the order behaviour can be plugged in using composition instead of inheritance.
Interface Segregation Principle (ISP)
Many client specific interfaces are better than one general purpose interface.
Some code that violates this principle will be easy to identify due to having interfaces with a lot of methods on. This principle compliments SRP, as you may see that an interface with many methods is actually responsible for more than one area of functionality.
But sometimes even an interface with just two methods could be split into two interfaces:
In this example, given that there are times when the
decode
method might not be needed, and also that a codec can probably be treated as either an encoder or a decoder depending upon where it’s used, it may be better to split the SimpleCodec
interface into an Encoder
and aDecoder
. Some classes may choose to implement both, but it will not be necessary for implementations to override methods they do not need, or for classes that only need anEncoder
to be aware that their Encoder
instance also implements decode
.
Dependency Inversion Principle (DIP)
Depend upon Abstractions. Do not depend upon concretions.
This code is dependent on a lot of specific implementation details: JDBC as a connection to a (relational) database; database-specific SQL; knowledge of the database structure; and so on. This does belong somewhere in your system, but not here where there are other methods that don’t need to know about databases. Better to extract a DAO or use the Repository pattern, and inject the DAO or repository into this service.
https://xmruibi.gitbooks.io/interview-preparation-notes/content/OOD/index.html
Open for extension but closed for modifications
The design and writing of the code should be done in a way that new functionality should be added with minimum changes in the existing code. The design should be done in a way to allow the adding of new functionality as new classes, keeping as much as possible existing code unchanged.
Example
class ShapeEditor{
void drawShape(Shape s){
if (s.m_type==1)
drawRectangle(s);
else if (s.m_type==2)
drawCircle(s);
};
void drawRectangle(Shape s);
void drawCircle(Shape s);
}
class Shape{}
class Rectangle extends Shape{}
class Circle extends Shape{}
//---> Every time if new shape added, we have to modify method in the editor class, which violates this rule!
// ---> change!!! write draw method in each shape concreted class
class Shape{ abstract void draw(); }
class Rectangle extends Shape{ public void draw() { /*draw the rectangle*/} }
class Circle extends Shape{ public void draw() { /*draw the circle*/} }
class ShapeEditor{
void drawShape(Shape s){ s.draw(); }
}
Dependency Inversion
The low level classes the classes which implement basic and primary operations(disk access, network protocols,...) and high level classes the classes which encapsulate complex logic(business flows, ...). The last ones rely on the low level classes.
High-level modules should not depend on low-level modules. Both should depend on abstractions.Abstractions should not depend on details. Details should depend on abstractions.
When this principle is applied it means the high level classes are not working directly with low level classes, they are using interfaces as an abstract layer.
Example
class WorkerWithTechOne{}
class Manager{
WorkerWithTechOne worker;
}
// --> what if add other workers with other techniques?
// --> abstract worker!
interface Worker{}
class WorkerWithTechOne implements Worker{}
class WorkerWithTechTwo implements Worker{}
class Manager{
Worker worker;
}
Interface Segregation
Clients should not be forced to depend upon interfaces that they don't use.
Instead of one fat interface, many small interfaces are preferred based on groups of methods, each one serving one submodule.
Example
interface work{
public void work();
// too much methods!
public void life();
public void rest();
}
// ---> change!!!
interface work{public void work();}
interface life{public void life();}
interface rest{public void rest();}
Single Responsibility
A class should have only one reason to change.
A simple and intuitive principle, but in practice it is sometimes hard to get it right.
This principle states that if we have 2 reasons to change for a class, we have to split the functionality in two classes. Each class will handle only one responsibility and on future if we need to make one change we are going to make it in the class which handle it. When we need to make a change in a class having more responsibilities the change might affect the other functionality of the classes.