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.