Take the 2-minute tour ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.

Based on the answer to my question on StackOverflow, I have ended up with the following code:

public class ColumnDataBuilder<T>
{
    public abstract class MyListViewColumnData
    {
        public string Name { get; protected set; }
        public int Width { get; protected set; }
        public ColumnType Type { get; protected set; }
        public delegate TOUT FormatData<out TOUT>(T dataIn);

        protected abstract dynamic GetData(T dataRow);

        public string GetDataString(T dataRow)
        {
            dynamic data = GetData(dataRow);
            switch (Type)
            {
                case ColumnType.String:
                case ColumnType.Integer:
                case ColumnType.Decimal:
                    return data.ToString();
                case ColumnType.Date:
                    return data.ToShortDateString();
                case ColumnType.Currency:
                    return data.ToString("c");
                    break;
                case ColumnType.Boolean:
                    var b = (bool)data;
                    if (b) return "Y";
                    else return "N";
                default:
                    throw new ArgumentOutOfRangeException();
            }
        }

    }

    public class MyListViewColumnData<TOUT> : MyListViewColumnData
    {

        public MyListViewColumnData(string name, int width, ColumnType type, FormatData<TOUT> dataFormater)
        {
            DataFormatter = x => dataFormater(x); // Per http://stackoverflow.com/a/1906850/298754
            Type = type;
            Width = width;
            Name = name;
        }
        public Func<T, TOUT> DataFormatter { get; protected set; }
        protected override dynamic GetData(T dataRow)
        {
            return DataFormatter(dataRow);
        }
    }
 }

This is called from a factory method (in ColumnDataBuilder) as

public MyListViewColumnData Create<TOUT>(string name, int width, ColumnType type, MyListViewColumnData.FormatData<TOUT> dataFormater)
{
    return new MyListViewColumnData<TOUT>(name, width, type, dataFormater);
}
public MyListViewColumnData Create(string name, int width, MyListViewColumnData.FormatData<DateTime> dataFormater)
{
    return new MyListViewColumnData<DateTime>(name, width, ColumnType.Date, dataFormater);
}
...

That, in turn, is called from my code as:

builder.Create("Date", 40, x => x.createdDate);

and

private ListViewItem CreateListViewItem<TDATA>(IEnumerable<ColumnDataBuilder<TDATA>.MyListViewColumnData> columns, TDATA rowData)
{
    var item = new ListViewItem();
    foreach (var col in columns)
    {
        item.SubItems.Add(col.GetDataString(rowData));
    }
    item.SubItems.RemoveAt(0); // We generate an extra SubItem for some reason.
    return item;
}

How can I refactor this so that I'm not using dynamic, but still preserve the syntax as it currently exists in the code?

share|improve this question
    
Alternatively, if this is a proper use for dynamic, that'd be good to know too... but I'm pretty sure it isn't. –  Bobson Dec 3 '12 at 16:52
    
Where does T come from? –  Jeff Vanzella Dec 3 '12 at 17:19
    
Your public abstract class MyListViewColumnData has no generic parameters while it has delegate TOUT FormatData<out TOUT>(T dataIn) declared. Please make sure you post a compilable code here –  almaz Dec 3 '12 at 17:30
    
@JeffVanzella - Good catch. I missed copying the class declaration. Edited it in. –  Bobson Dec 3 '12 at 17:33
    
@almaz - I think this will now compile. If it doesn't, I'll go back and post a larger block, but I am trying to keep it down to the relevant portions. –  Bobson Dec 3 '12 at 17:37
show 1 more comment

1 Answer

up vote 1 down vote accepted

I don't think you need MyListViewColumnData class there, I would replace it with interface and move the GetDataString implementation to MyListViewColumnData<TOut>. And you don't need dynamic here, just use object instead (yes, it will use boxing for most cases except strings, but it's more efficient than dynamics).

public class ColumnDataBuilder<T>
{
    public interface IMyListViewColumnData
    {
        string Name { get; }
        int Width { get; }
        ColumnType Type { get; }
        string GetDataString(T dataRow);
    }

    public delegate TOut FormatData<out TOut>(T dataIn);

    public class MyListViewColumnData<TOut> : IMyListViewColumnData
    {
        public string Name { get; private set; }
        public int Width { get; private set; }
        public ColumnType Type { get; private set; }
        private readonly FormatData<TOut> _dataFormatter;

        public MyListViewColumnData(string name, int width, ColumnType type, FormatData<TOut> dataFormater)
        {
            _dataFormatter = dataFormater;
            Type = type;
            Width = width;
            Name = name;
        }
        public string GetDataString(T dataRow)
        {
            object data = _dataFormatter(dataRow);

            switch (Type)
            {
                case ColumnType.String:
                case ColumnType.Integer:
                case ColumnType.Decimal:
                    return data.ToString();
                case ColumnType.Date:
                    return ((DateTime)data).ToShortDateString();
                case ColumnType.Currency:
                    return ((decimal)data).ToString("c");
                case ColumnType.Boolean:
                    return (bool)data ? "Y" : "N";
                default:
                    throw new ArgumentOutOfRangeException();
            }
        }
    }

    public IMyListViewColumnData Create<TOut>(string name, int width, ColumnType type, FormatData<TOut> dataFormater)
    {
        return new MyListViewColumnData<TOut>(name, width, type, dataFormater);
    }

    public IMyListViewColumnData Create(string name, int width, FormatData<DateTime> dataFormater)
    {
        return new MyListViewColumnData<DateTime>(name, width, ColumnType.Date, dataFormater);
    }
}

public enum ColumnType
{
    String,
    Integer,
    Decimal,
    Date,
    Currency,
    Boolean
}

Update In comments it was asked if you can extract interface for ColumnDataBuilder. Of course you can :), and the easiest way would be to use "Extract interface" refactoring from ReSharper :). If you still don't use it you'll have to do that manually (move the IMyListViewColumnData and FormatData<TOut> declarations out of ColumnDataBuilder<T> first):

public interface IColumnDataBuilder<Tin>
{
    IMyListViewColumnData Create<TOut>(string name, int width, ColumnType type, FormatData<TOut> dataFormater);
    IMyListViewColumnData Create(string name, int width, FormatData<DateTime> dataFormater);
}
share|improve this answer
    
I knew there had to be a better way. Thanks! –  Bobson Dec 3 '12 at 18:33
    
Just a followup: Is there any way to extract an interface of ColumnDataBuilder<>, so that I can store it without knowing the type? Or is that asking too much of the compiler? –  Bobson Dec 3 '12 at 18:47
    
@Bobson see updated answer –  almaz Dec 3 '12 at 18:58
    
I'm not sure that addresses the goal, though. I have another class (the ListView subclass which this is handling the data for), which could have any datatype for Tin there. And I can't make it generic for other reasons, or this would be a lot simpler. So what type do I make that property? Or could I leave off the <Tin> and get this scenario working? –  Bobson Dec 3 '12 at 19:03
    
you should have told how you want it to work first :). If you want to write a general logic that doesn't know about specific types upfront in may be a good idea just to stop using generics... At what time ListView gets to know the type T? Where is the code that calls ColumnDataBuilder<T>.Create located? Does that code know about specific types? It's probably better to open a new question since we've moved quite far from original one... –  almaz Dec 3 '12 at 19:09
show 2 more comments

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

Not the answer you're looking for? Browse other questions tagged or ask your own question.