Constructor and Initialization

I'd like to review a code excerpt from one real application. Basically it converts a Datatable into a List of entities and then used for binding to UI control. I will address in detail two features involved in the code, and suggest how to refactor the code better.

The code excerpt

public List<Entity> CalcualteFields(DataTable dt)
{
var query =
from d in dt.AsEnumerable()
select new
{
TRANSIT_ID = d.Field<int>("TRANSIT_ID"),
DFR_AMOUNT = d.Field<decimal>("DFR_AMOUNT"),
DFR_AMOUNT_ADJUST = d.Field<decimal>("DFR_AMOUNT_ADJUST"),
eGL_AMOUNT = d.Field<decimal>("eGL_AMOUNT"),
eGL_AMOUNT_ADJUST = d.Field<decimal>("eGL_AMOUNT_ADJUST"),
DFR_VAR = (d.Field<decimal>("DFR_AMOUNT") + d.Field<decimal>("DFR_AMOUNT_ADJUST")) - (d.Field<decimal>("eGL_AMOUNT") + d.Field<decimal>("eGL_AMOUNT_ADJUST")),
DOD_eGL_VAR = d.Field<decimal>("eGL_AMOUNT") - ((d.Field<decimal>("eGL_AMOUNT_LD")) + ((d.Field<decimal>("eGL_AMOUNT_ADJUST_LD"))))
};

List<DFReGLData> DFReGLList = new List<DFReGLData>();
foreach (var d in query)
{
DFReGLData DFReGLD = new DFReGLData();
DFReGLD.TRANSIT_ID = d.TRANSIT_ID;
DFReGLD.DFR_AMOUNT = d.DFR_AMOUNT;
DFReGLD.DFR_AMOUNT_ADJUST = d.DFR_AMOUNT_ADJUST;
DFReGLD.eGL_AMOUNT = d.eGL_AMOUNT;
DFReGLD.eGL_AMOUNT_ADJUST = d.eGL_AMOUNT_ADJUST;
DFReGLD.DFR_VAR = d.DFR_VAR;
DFReGLD.DOD_eGL_VAR = d.DOD_eGL_VAR;
DFReGLList.Add(DFReGLD);
}

return DFReGLList;
}

Arguably, transforming a Datatable into a list of objects and then binding to a UI control is a good style, at least performance wise it is not, especially given the fact that the UI control support Datatable binding.

Anyways, what I care about here are the two good features: Field() function of DataRow and a new way of object initialization.

Field function

Before I can use this function, I have to convert the value before assigning to a variable, as shown below,

int TRANSIT_ID = Int32.Convert(row["TRANSIT_ID"]);

If I need to ensure the conversion is valid and not causing exceptions, I also need to put above code into a try-catch block, or use TryParse function. Now things get much easier, as shown in following code, the two steps are isolated.

int TRANSIT_ID = row.Field("TRANSIT_ID");

Constructor and Initialization

Normally, I create an instance by calling the constructor and then initialized it, as demonstrated below:

Entity en = new Entity(p1, p2);
en.Name = "bla";
en.Title = "blabla";

From C# 3.0, initialization can be put in one statement of the construction:

Entity en = new Entity(p1, p2){Name="bla", Title="blabla"};

Even simpler, I don't need to state explicitely the target type, as shown below, because the compiler knows it from the context.

var en = new Entity(p1, p2){Name="bla", Title="blabla"};

Of course, the properties should be accessible by defining accessor like:

public string Name{ get; set; }


Make code better

With the new way of initialization, above code excerpt can be better if I generate the list in one loop rather than two:

public List<Entity> GetEntityList(DataTable dt)
{
var query =
from d in dt.AsEnumerable()
select new Entity
{
TRANSIT_ID = d.Field<int>("TRANSIT_ID"),
DFR_AMOUNT = d.Field<decimal>("DFR_AMOUNT"),
DFR_AMOUNT_ADJUST = d.Field<decimal>("DFR_AMOUNT_ADJUST"),
eGL_AMOUNT = d.Field<decimal>("eGL_AMOUNT"),
eGL_AMOUNT_ADJUST = d.Field<decimal>("eGL_AMOUNT_ADJUST"),
DFR_VAR = (d.Field<decimal>("DFR_AMOUNT") + d.Field<decimal>("DFR_AMOUNT_ADJUST")) - (d.Field<decimal>("eGL_AMOUNT") + d.Field<decimal>("eGL_AMOUNT_ADJUST")),
DOD_eGL_VAR = d.Field<decimal>("eGL_AMOUNT") - ((d.Field<decimal>("eGL_AMOUNT_LD")) + ((d.Field<decimal>("eGL_AMOUNT_ADJUST_LD"))))
};

return query.ToList();
}