Way(s) to avoid continuous, similar conditional blocks

Want to know if there is a better approach to handle multiple, similar conditional statements and actions such as in the example snippet below.

private void AddCommonDictionaryItemsForAllAttributes(MyCustomType dc, string statusFlag)
{
    if (dc.xmlAttributes == null) {
        dc.xmlAttributes = new Dictionary<string, string>();
    }
    dc.xmlAttributes.Add(Constant.CD_1, statusFlag);
    dc.xmlAttributes.Add(Constant.CD_2, statusFlag);
    dc.xmlAttributes.Add(Constant.CD_3, statusFlag);
    if (dc.primaryZone != null) {
        dc.xmlAttributes.Add(Constant.CD_4, statusFlag);
    }
    if (dc.Mgr1 != null) {
        dc.xmlAttributes.Add(Constant.CD_10, statusFlag);
    }
    if (dc.Mgr2 != null) {
        dc.xmlAttributes.Add(Constant.CD_11, statusFlag);
    }
    if (dc.Mgr3 != null) {
        dc.xmlAttributes.Add(Constant.CD_5, statusFlag);
    }    
    if (dc.Producer != null) {
        dc.xmlAttributes.Add(Constant.CD_6, statusFlag);
    }
    if (dc.CountTest > 0) {
        dc.xmlAttributes.Add(Constant.CD_7, statusFlag);
    }
    if 开发者_Python百科(dc.List1 != null && dc.List1.Count > 0) {
        dc.xmlAttributes.Add(Constant.CD_8, statusFlag);
    }
    if (dc.List2 != null && dc.List2.Count > 0) {
        dc.xmlAttributes.Add(Constant.CD_9, statusFlag);
    }
}

The if conditions and the addition to the dictionary operation seems to me as redundant so looking out for more efficient and elegant ways to code this.

Thanks!

Update: I am using .NET 3.5

You could create a helper type, which provides a test to be performed on an instance of MyCustomType, and the key to use in the xmlAttributes dictionary:

class Rule
{
    private readonly Predicate<MyCustomType> _test;
    private readonly string _key;

    public Predicate<MyCustomType> Test { get { return _test; } }
    public string Key { get { return _key;  } }

    public Rule(Predicate<MyCustomType> test, string key)
    {
        _test = test;
        _key = key;
    }
}

You can then create a set of these rules, and enumerate them:

    private void AddCommonDictionaryItemsForAllAttributes(MyCustomType dc, string statusFlag)
    {

        var rules = new Rule[]
        {
            new Rule(x => x.Mgr1 != null, Constant.CD_4),
            new Rule(x => x.Mgr2 != null, Constant.CD_10),
            //...snip...
            new Rule(x => x.List2 != null && x.List2.Count > 0, Constant.CD_9)
        };

        foreach(var rule in rules.Where(r => r.Test(dc)))
            dc.xmlAttributes.Add(rule.Key, statusFlag);
    }

One option would be to have some sort of list of conditions and the constants represented by those conditions. For example:

var list = new List<Tuple<Predicate<MyCustomType>, string>>
{
    Tuple.Create(dc => true, Constant.CD_1),
    Tuple.Create(dc => true, Constant.CD_2),
    Tuple.Create(dc => true, Constant.CD_3),
    Tuple.Create(dc => dc.primaryZone != null, Constant.CD_4),
    Tuple.Create(dc => dc.Mgr1 != null, Constant.CD_5),
    // etc
};

Then you could just iterate over the list, setting the relevant to status in the dictionary whenever the predicate was true:

foreach (var tuple in list)
{
    if (tuple.Item1(dc))
    {
        dc.xmlAttributes.Add(tuple.Item2, statusFlag);
    }
}

Note that you can set the list up statically once and then reuse it everywhere, as the list itself doesn't change.

You can do it in for loop, because you can cast into to enum.

Consider encapsulating attributes collection inside YourCustomClass. This will protect your attributes from accidental changing, and it will move attributes filling logic toward the data it belongs to.

Benefits:

• You can change attributes filling implementation anytime, without changing clients (conditions, predicates collection etc).
• Much more clean client
• Easier maintenance

So, even with your default implementation usage will look like this:

dc.SetStaus(string statusFlag)

And all the dirty job will be done inside dc (BTW I advice to use enum CD instead of constants, but it's up to you):

public void SetStatus(string statusFlag)
{
    if (_xmlAttributes == null)
        _xmlAttributes = new Dictionary<CD, string>();

    _xmlAttributes.Add(CD.CD_1, statusFlag);
    _xmlAttributes.Add(CD.CD_2, statusFlag);
    _xmlAttributes.Add(CD.CD_3, statusFlag);

    if (_primaryZone != null)
        _xmlAttributes.Add(CD.CD_4, statusFlag);

    if (_mgr1 != null)
        _xmlAttributes.Add(CD.CD_10, statusFlag);

    if (_mgr2 != null)
        _xmlAttributes.Add(CD.CD_11, statusFlag);

    if (_mgr3 != null)
        _xmlAttributes.Add(CD.CD_5, statusFlag);

    if (_producer != null)
        _xmlAttributes.Add(CD.CD_6, statusFlag);

    if (_countTest > 0)
        _xmlAttributes.Add(CD.CD_7, statusFlag);

    if (_list1 != null && _list1.Count > 0)
        _xmlAttributes.Add(CD.CD_8, statusFlag);

    if (_list2 != null && _list2.Count > 0)
        _xmlAttributes.Add(CD.CD_9, statusFlag);
}

After that you can refactor implementation easily:

private Dictionary<CD, Func<bool>> _statusSetConditions;

public MyCustomType()
{
    _statusSetConditions = new Dictionary<CD, Func<bool>>();
    _statusSetConditions.Add(CD.CD_1, () => true);
    _statusSetConditions.Add(CD.CD_2, () => true);
    _statusSetConditions.Add(CD.CD_3, () => true);
    _statusSetConditions.Add(CD.CD_4, () => _primaryZone != null);
    ...
    _statusSetConditions.Add(CD.CD_11, () => _mgr2 != null);
}

public void SetStatus(string statusFlag)
{
    if (_xmlAttributes == null)
        _xmlAttributes = new Dictionary<CD, string>();

    foreach (CD cd in Enum.GetValues(typeof(CD)))
        AddStatusAttribute(cd, statusFlag);
}

private void AddStatusAttribute(CD cd, string statusFlag)
{
    Func<bool> condition;

    if (!_statusSetConditions.TryGetValue(cd, out condition))
        return; // or throw exception

    if (condition())
        _xmlAttributes.Add(cd, statusFlag);
}  

And client still just calls to dc.SetStatus(statusFlag);

Maybe after encapsulating this status-set logic, you will just save status in field of YourCustomClass.

Meh. It's not really redundant, unless you want to consider something like java reflection. Consider helper methods:

void addIfOk(int test, MyCustomType dc, String attr, string statusFlag) {
  if(test!=0) dc.xmlAttributes.Add(attr, statusFlag);
}
void addIfOk(Object test, MyCustomType dc, String attr, string statusFlag) {
  if(test!=null) dc.xmlAttributes.Add(attr, statusFlag);
}
void addIfOk(Collection test, MyCustomType dc, String attr, string statusFlag) {
  if(test!=null&&!test.isEmpty()) dc.xmlAttributes.Add(attr, statusFlag);
}

The code then becomes:

    addIfOk(dc.Mgr1, dc, Constant.CD_10, statusFlag);
    addIfOk(dc.Mgr2, dc, Constant.CD_11, statusFlag);
    addIfOk(dc.Mgr3, dc, Constant.CD_5, statusFlag);
    addIfOk(dc.Producer, dc, Constant.CD_5, statusFlag);

and so on. Perhaps this would make more sense as a method inside your custom type: setXmlStatusAttributes(statusfFlag)

There are two ways: 1. Use switch case 2. Use Ternary operator

both will make your code to look clean, however in your case switch case doesn't work.