How to refactor this code?

Question

how can i refactor this code down to one method or something?

            if (!string.IsNullOrEmpty(_gridModel.Header))
                _gridModel.Header += ",";
            if (item != null)
                _gridModel.Header += item.Header;

            if (!string.IsNullOrEmpty(_gridModel.Width))
                _gridModel.Width += ",";
            if (item != null)
                _gridModel.Width += item.Width;

            if (!string.IsNullOrEmpty(_gridModel.Align))
                _gridModel.Align += ",";
            if (item != null)
                _gridModel.Align += item.Align;

            if (!string.IsNullOrEmpty(_gridModel.Filter))
                _gridModel.Filter += ",";
            if (item != null)
                _gridModel.Filter += item.Filter;

            if (!string.IsNullOrEmpty(_gridModel.Type))
                _gridModel.Type += ",";
            if (item != null)
                _gridModel.Type += item.Type;

            if (!string.IsNullOrEmpty(_gridModel.Sort))
                _gridModel.Sort += ",";
            if (item != null)
                _gridModel.Sort += item.Sort;

Alan Jackson · Accepted Answer · 2009-09-29 20:16:48Z

15

To start, refactor the logic into a function.

_gridModel.Header = AppendItem(_gridModel.Header, item == null ? null : item.Header);
_gridModel.Width = AppendItem(_gridModel.Width, item == null ? null : item.Width);
...
...

string AppendItem(string src, string item)
{
 if (! string.IsNullOrEmpty(src))
  src += ",";
 if (! string.IsNullOrEmpty(item))
  src += item;
 return src;
}

A good next step could be to use reflection and properties:

Edit: Fleshed out reflection solution, haven't actually debugged it yet though.

AppendProperties(_gridModel, item, "Header", "Width", "Align", ...)

void AppendProperty(object gridmodel, object item, params string[] propNames)
{
    foreach (string propName in propNames)
        AppendProperties(gridmodel, item, propName);
}

void AppendProperties(object gridmodel, object item, string propName)
{
    PropertyInfo piGrid = gridmodel.GetType().GetProperty(propName);
    if (piGrid != null && piGrid.PropertyType == typeof(string))
    {
        piGrid.SetValue(gridmodel, 
            piGrid.GetValue(gridmodel, null).ToString() + ",", null);
    }

    if (item == null) return;
    PropertyInfo piItem = item.GetType().GetProperty(propName);
    if (piItem != null)
    {
        piGrid.SetValue(gridmodel, 
            piGrid.GetValue(gridmodel, null).ToString() 
            + piItem.GetValue(item, null).ToString(), 
            null);
    }
}

edited Sep 29, 2009 at 20:16

answered Sep 29, 2009 at 16:57

Alan Jackson

6,5212 gold badges33 silver badges32 bronze badges

Sign up to request clarification or add additional context in comments.

5 Comments

csharptest.net Over a year ago

+1 close enough, I might use a (ref string src, ...) and not return the value.

Alan Jackson Over a year ago

Would be faster, but I tend to default to no side effects when possible.

eglasius Over a year ago

AppendItem(_gridModel.Header, item.Header); <- NullReferenceException when calling item.Header

Jeremy Roberts Over a year ago

I like your first step, I think it is actually better then the accepted answer. However, the use of reflection and magic strings in this context is, in my opinion, overkill.

Alan Jackson Over a year ago

Freddy: added check for item is null when calling the function, thanks.

Mkkt Bkkt · Accepted Answer · 2009-09-30 22:20:21Z

7

Assuming you have .NET 3.5:

string Filter(string input, SomeType item, Func<SomeType, string> extract)
{
    if (!String.IsNullOrEmpty(input))
    {
        if (item == null) return ",";
        else return "," + extract(item);
    }
}

_gridModel.Header += Filter(_gridModel.Header, item, i => i.Header);
_gridModel.Width += Filter(_gridModel.Width, item, i => i.Width);
_gridModel.Align += Filter(_gridModel.Align, item, i => i.Align);

// etc...

edited Sep 30, 2009 at 22:20

answered Sep 29, 2009 at 16:59

Mkkt Bkkt

122k40 gold badges154 silver badges177 bronze badges

2 Comments

sooniln Over a year ago

Depending on your usage, with so many appends, it may give you better performance to use StringBuilder

Mkkt Bkkt Over a year ago

Oh yeah if it's in a loop it might make more sense to pass a stringbuilder around instead of concatenation.

Collectives™ on Stack Overflow

How to refactor this code?

2 Answers 2

5 Comments

2 Comments

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

2 Answers 2

5 Comments

2 Comments

Your Answer

Sign up or log in

Post as a guest

Related