Best Practices, same functionality from 1 method and also from 2 methods
-
27-04-2021 - |
Question
I have following scenario. Some people use approach 1 and some use approach 2. Both approaches have same functionality lock or unlock news against news id in news table. what you suggest,which one is better and why?
Note:I use void return type instead of bool for simplicity so please ignore this
Approach 1:
public void LockNews(long newId)
{
......
}
public void UnlockNews(long newId)
{
...
}
Approach 2:
public void LockUnlockNews(long newId,bool Islock)
{
......
}
La solution
Approach 1, for me, because IMHO a method should represent a single operation. In the interest of writing testable code or any form of automated testing, it would maintain a clear separation and make a lot more sense.
Approach 2 is leaning towards a "does everything" operation, which IMHO should be avoided.
Autres conseils
I prefer Approach 1. It makes it clear what is going on. If you use approach 2 and call
LockUnlockNews(42, true);
It is not immediately clear whether this is a lock or an unlock. Just to throw some fuel on the fire: If you changed the bool to an enum or a const, then my argument is null and void.
LockUnlockNews(42, LOCK);
is just as clear as
LockNews(42);
First approach.
Your method is a Command and should be as explicit as possible. I would even ask you why someone but News itself know how to lock/unlock? For me News should be responsible for it:
var news = GetNewsSomehow(newsId);
news.Lock();
news.Unlock();
Makes more sense, doesn't it? :) You clearly see what your objects are and what behaviors they have. This is what is called encapsulation in OOP.
I combine both approaches generally like below:
Public methods provide clear interface and are more testable:
public void LockNews(long newId)
{
LockUnlockNews(newId, true);
}
public void UnlockNews(long newId)
{
LockUnlockNews(newId, false);
}
Private method does actual business and increases modularity managing things from one location:
private void LockUnlockNews(long newId,bool Islock)
{
......
}