CODEWITHSUNDEEP
c#asp.netcoding-styleapplication-design
Ali
Ali

Reputation: 3615

Best Practices, same functionality from 1 method and also from 2 methods

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)
{
 ......
}

Upvotes: 2

Views: 141

Answers (4)

mmdemirbas
mmdemirbas

Reputation: 9158

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)
{
    ......
}

Upvotes: 0

Alexey Raga
Alexey Raga

Reputation: 7525

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.

Upvotes: 1

John3136
John3136

Reputation: 29266

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);

Upvotes: 1

csharpnumpty
csharpnumpty

Reputation: 3723

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.

Upvotes: 3

Related Questions