Help refactor my C# code to be more maintainable and to use best coding practices

Question

I have this C# class structure that I would like to refactor to use best coding standards (use interfaces/abstract classes) so it can be more maintainable and reusable. The code as it is right now isn't awful, but it's not ideal.

I have a series of TableItemGroup classes: AccountTableItemGroup, PendingVoteTableItemGroup, and RequestingVoteTableItemGroup. Each TableItemGrup contains a string SectionName and a List for its corresponding TableItem ...as such:

public class AccountTableItemGroup {
    public string SectionName { get; set; }

    public List<AccountTableItem> Items
    {
        get { return this._items; }
        set { this._items = value; }
    }        
    public List<AccountTableItem> _items = new List<AccountTableItem>();

    public AccountTableItemGroup()
    {
    }
}

In the future there will be many more TableItemGroups and if they are all the same except for the List part, I don't want to have to copy the code and create a new Group every time and make that small change. I know there must be a better way. I would like to keep using the List<> generics so I don't have to cast anything later though.

The other part are the TableItems. I have AccountTableItem, PendingVoteTableItem, and RequestingVoteTableItem. The TableItems are different from each other, but they each share three common strings -- TitleLabel, DetailLabel, and ImageName. But after that, each TableItem may or may not have additional properties or methods along with it ..as such:

public class AccountTableItem
{
    public string TitleLabel { get; set; }

    public string DetailLabel { get; set; }

    public string ImageName { get; set; }

    public bool SwitchSetting { get; set; }

    public AccountTableItem()
    {
    }
}

So my question to all of you is, how do I redefine my class structure to allow for as much reuse of code as possible and to use best coding standards?

I was thinking of having an abstract TableItem class or use an interface for the TableItemGroup? I know that using an interface or an abstract class is best for coding standards, but I don't see how it would cut down on the amount of code I will have?

Thanks a lot for any help.

Grozz · Answer 1 · 2011-08-10T12:29:42.960

Abstract away your table item adding necessary fields to the interface or base class:

    interface ITableItem // or just a simple or abstract class
    {
        // common fields go here
    }

Then can you make your item group generic with a constraint on generic parameter.

    public class ItemGroup<T> where T: ITableItem
    {
        public string SectionName { get; set; }

        public List<T> Items { get; private set; }

        public ItemGroup()
        {
            Items = new List<T>();
        }
    }

Tim Lloyd · Accepted Answer · 2011-07-29T14:10:50.677

Consider using generics to represent the TableItemGroup container, and make a base class for your TableItem, which you can inherit from for specific types of table item. If you inherit directly from List<T>, then you can treat your item group as a collection without having to use the Items property as in your existing design.

There's not much point in using interfaces for these sorts of types. As they stand they are data classes so have no behavior. If they had behavior, using interfaces would make sense as you would then be able to change implementations and so vary behavior.

public class TableItemGroup<T> : List<T> where T : TableItem
{
    public TableItemGroup(string sectionName)
    {
        SectionName = sectionName;
    }

    public string SectionName { get; private set; }
}

public class TableItem
{
    public string TitleLabel { get; set; }

    public string DetailLabel { get; set; }

    public string ImageName { get; set; }
}

public class AccountTableItem : TableItem
{
    public bool SwitchSetting { get; set; }
}

Now that we have a generic TableItemGroup container, you can re-use this for all TableItem types. Having a base class for TableItem again gives you some re-use.

var items = new TableItemGroup<AccountTableItem>("Accounts");

items.Add(new AccountTableItem { SwitchSetting = true });

I really like this method. Especially the TableItemGroup inheriting from List. Question: Do I have to provide implementations for all of the TableItems even if they do not add additional properties? Like a "public class OptionsTableItem: TableItem { }" ...Just leave it blank, or do I not have to implement it at all? Thanks. — apexdodge, Jul 29 '11 at 14:06
@Apex If you have cases where a `TableItem` would provide all the required properties, then remove the `abstract` modifier. This way you can use `TableItem` directly without having to inherit. — Tim Lloyd, Jul 29 '11 at 14:08
thanks Chibacity. Huge help in my quest to become a better programmer :) — apexdodge, Jul 29 '11 at 14:12

score 1 · Answer 3 · answered Jul 29 '11 at 13:34

1

Unless you want users to be able to add and remove new lists at will, you should make the setter on the items list protected. Users will still be able to add and remove items, but not create a reference to a new list.

answered Jul 29 '11 at 13:34

John Kraft

6,811
4
37
53

Help refactor my C# code to be more maintainable and to use best coding practices

3 Answers3