tags:

views:

241

answers:

3

I have this repeated code below and I am assuming that this can be consolidated but if you notice each dictionary is different generic dictionary:

dictionary1 is of type

Dictionary<int, ContinuousIntegrationSolution>

whereas dictionary2 is of type:

Dictionary<int, BugTracker>

      DataTable dt = GetDataTable("CI");
        for (int i = 0; i < dt.Rows.Count; i++)
        {
            DataRow dr = dt.Rows[i];
            int id = Convert.ToInt32(dr["id"]);
            string name = dr["name"].ToString();
            _dictionary1[id] = new ContinuousIntegrationSolution(){Name = name};
        }

        DataTable dt1 = GetDataTable("Bug_Tracking");

        for (int i = 0; i < dt1.Rows.Count; i++)
        {
            DataRow dr = dt1.Rows[i];
            int id = Convert.ToInt32(dr["id"]);
            string name = dr["name"].ToString();
            _dictionary2[id] = new BugTracker() { Name = name };
        }

        DataTable dt2 = GetDataTable("SDLC");

        for (int i = 0; i < dt2.Rows.Count; i++)
        {
            DataRow dr = dt2.Rows[i];
            int id = Convert.ToInt32(dr["id"]);
            string name = dr["name"].ToString();
            _dictionary3[id] = new SDLCProcess() { Name = name };
        }


NOTE: I have fixed the few typos that were mentioned below.

+13  A: 
public interface INameable
{
    string Name {get;set;}
}

public static IDictionary<int, T> ReadTable<T>(string tableName)
    where T : INameable, new()
{
    DataTable dt = GetDataTable(tableName);
    var dictionary = new Dictionary<int, T>();

    for (int i = 0; i < dt.Rows.Count; i++)
    {
        DataRow dr = dt.Rows[i];
        int id = Convert.ToInt32(dr["id"]);
        string name = dr["name"].ToString();
        dictionary[id] = new T() { Name = name };
    }
    return dictionary;
}

If you have c# 4.0 dynamic you can avoid the INameable for some (minor) loss of typesafety

An alternate, similar in vein to Hakon's answer but without exposing the dictionary is

public IDictionary<int,T> ReadTable<T>(
    string tableName, Action<T, string> onName) 
    where T : new()
{
    var dictionary = new Dictionary<int,T>();
    DataTable table = GetDataTable(tableName);
    foreach (DataRow row in table.Rows) 
    {
        int id = Convert.ToInt32(row["id"]);
        string name = row["name"].ToString();
        var t = new T();
        onName(t, name);
        dictionary[id] = t;
    }
    return dictionary;
}

which is then consumed like so:

var ci   = ReadTable<ContinuousIntegrationSolution>("CI", 
              (t, name) => t.Name = name);  
var bt   = ReadTable<BugTracker >("Bug_Tracking", 
              (t, name) => t.Name = name);  
var sdlc = ReadTable<SDLCProcess>("SDLC", 
              (t, name) => t.Name = name);

An alternate, more flexible approach, but still reasonably simple to use at the call site due to type inference would be:

public IDictionary<int,T> ReadTable<T>(string tableName, Func<string,T> create) 
{
    DataTable table = GetDataTable(tableName);
    var dictionary = new Dictionary<int,T>()
    foreach (DataRow row in table.Rows) 
    {
        int id = Convert.ToInt32(row["id"]);
        string name = row["name"].ToString();
        dictionary[id] = create(name);
    }
    return dictionary;
}

which is then consumed like so:

var ci   = ReadTable("CI", 
               name => new ContinuousIntegrationSolution() {Name = name});  
var bt   = ReadTable("Bug_Tracking", 
               name => new BugTracker() {Name = name});
var sdlc = ReadTable("SDLC", 
               name => new SDLCProcess() {Name = name});

If you were to go with the lambda approach I would suggest the latter.

ShuggyCoUk
It could be improved further by using a foreach.
Mark Byers
I would agree, but that would change the semantics of the program as supplied (a subtle change I admit) so I tend not to *correct such things when it is unnecessary.
ShuggyCoUk
i am getting an error with this code saying: Error 2 Constraints are not allowed on non-generic declarations
ooo
@oo: That's just a case of making the method generic. I'll fix it.
Jon Skeet
a few more issues... it must be IDictionary<int, T> (not string, T)
ooo
also, new() must be after INameable as its complaining that it must be the last item
ooo
fixed the ordering - that will teach me to write code without a compiler to hand.
ShuggyCoUk
A: 

Put it in a function with a factory method to instantiate the id object. Something like this...

public delegate T CreateObjectDelegate<T>(string name);
public static void ProcessDataTable<T>(DataTable dt, Dictionary<int, T> dictionary, CreateObjectDelegate<T> createObj)
{
    for (int i = 0; i < dt.Rows.Count; i++)
    {
        DataRow dr = dt.Rows[i];
        int id = Convert.ToInt32(dr["id"]);
        string name = dr["name"].ToString();
        dictionary[id] = createObj(name);
    }
}

static void Main(string[] args)
{
    var dt = new DataTable();
    var dictionary = new Dictionary<int, BugTracker>();
    ProcessDataTable<BugTracker>(dt, dictionary, (name) => { return new BugTracker() { Name = name }; });
}
Brian
+3  A: 

I would not use an interface like it has been suggested but rather use a lambda function to do the assignment similar to this:

public void ReadTable(string tableName, Action<int, string> _setNameAction) {
    DataTable table = GetDataTable(tableName);
    foreach (DataRow row in table.Rows) {
        int id = Convert.ToInt32(row["id"]);
        string name = row["name"].ToString();
        _setNameAction(id, name);
    }
}

And call the method like this:

ReadTable("CI", (id, name)
    => _dictionary1[id] = new ContinuousIntegrationSolution{Name = name});  
ReadTable("Bug_Tracking", (id, name) 
    => _dictionary2[id] = new BugTracker { Name = name });  
ReadTable("SDLC", (id, name) 
    => _dictionary3[id] = new SDLCProcess { Name = name });
HakonB
this code doesn't work. where is "id" being passed back into the Action delegate ??
ooo
I just figured that out seconds ago and have updated it now
HakonB