Refactor help c#

Question

I have several hundred lines of code like this:

if (c.SomeValue == null || c.SomeProperty.Status != 'Y')
{
    btnRecordCall.Enabled = false;
}

if (c.SomeValue == null || (c.SomeProperty.Status != 'Y' &&
    c.SomeOtherPropertyAction != 'Y'))
{
    btnAddAction.Enabled = false;
}

if (c.SomeValue == null || c.SomeProperty.Processing != 'Y')
{
    btnProcesss.Enabled = false;
}

How can I refactor this correctly? I see that the check 'c.SomeValue == null' is being called every time, but it is included with other criteria. How can I possibly eliminate this duplicate code?

Answer 1

I would use the specification pattern, and build composite specifications that map to a proper Enabled value.

The overall question you want to answer is whether some object c satisfies a given condition, which then allows you to decide if you want something enabled. So then you have this interface:

interface ICriteria<T>
{
    bool IsSatisfiedBy(T c);
}

Then your code will look like this:

ICriteria<SomeClass> cr = GetCriteria();

btnAddAction.Enabled = cr.IsSatisfiedBy(c);

The next step is to compose a suitable ICriteria object. You can have another ICriteria implementation, (in additon to Or and And), called PredicateCriteria which looks like this:

class PredicateCriteria<T>  : ICriteria<T>
{
    public PredicateCriteria(Func<T, bool> p) {
        this.predicate = p;
    }

    readonly Func<T, bool> predicate;

    public bool IsSatisfiedBy(T item) {
        return this.predicate(item);
    }
}

One instance of this would be:

var c = new PredicateCriteria<SomeClass>(c => c.SomeValue != null);

The rest would be composition of this with other criteria.

Answer 2

If you don't want to do much refactoring, you can easily pull the null check out.

if (c.SomeValue == null)
{
    btnRecordCall.Enabled = false;
    btnAddAction.Enabled = false;
    btnProcesss.Enabled = false;
}
else
{
    if(c.SomeProperty.Status != 'Y')
    {
        btnRecordCall.Enabled = false;
    }

    if((c.SomeProperty.Status != 'Y') &&
       (c.SomeOtherPropertyAction != 'Y'))
    {
        btnAddAction.Enabled = false;
    }

    if(c.SomeProperty.Processing != 'Y')
    {
        btnProcesss.Enabled = false;
    }
}

If you're looking to refactor instead of shuffle, the wall of boolean testing could be moved in to methods/extension methods of whatever class your object c is an instance of - that way you could say

btnRecordCall.Enabled = c.IsRecordCallAllowed();