How to write it better?

Question

Take a look at this code:

IList<IHouseAnnouncement> list = new List<IHouseAnnouncement>();
var table = adapter.GetData(); //get data from repository object -> DataTable

if (table.Rows.Count >= 1)
{
    for (int i = 0; i < table.Rows.Count; i++)
    {
        var anno = new HouseAnnouncement();
        anno.Area = float.Parse(table.Rows[i][table.areaColumn].ToString());
        anno.City = table.Rows[i][table.cityColumn].ToString();
        list.Add(anno);
    }
  }
  return list;

Is this the best way to write this in less code and better quality (should be :-))? Perhaps using lambda (but let me know how)?

Thanks in advance!

Answer 1

Just FYI, you never add a new one HouseAnnouncementto your list, and your loop will never run for the last line, but I assume that these are errors in the example, and not in your actual code.

You can do something like this:

return adapter.GetData().Rows.Cast<DataRow>().Select(row =>
    new HouseAnnouncement()
    {
        Area = Convert.ToSingle(row["powierzchnia"]),
        City = (string)row["miasto"],
    }).ToList();

I usually read for brevity, but I feel that it is pretty readable.

, DataTable table.powierzchniaColumn , , , ( , , ).

, , :

using (var table = adapter.GetData())
{
    return table.Rows.Cast<DataRow>().Select(row =>
        new HouseAnnouncement()
        {
            Area = Convert.ToSingle(row[table.powierzchniaColumn]),
            City = (string)row[table.miastoColumn],
        }).ToList();
}

, , .

Answer 2

- Linq:

var table = adapter.GetData();
var q = from row in table.Rows.Cast<DataRow>()
        select new HouseAnnouncement() 
           { Area = float.Parse(row[table.areaColumn].ToString()),
             City = row[table.cityColumn].ToString()
           };
return q.ToList();

Answer 3

"if statement" . "for" .

, "for" , 1. , , . , "-1":

for (int i = 0; i < table.Rows.Count; i++)

Answer 4

, -, " ":

for (int i = 0; i < table.Rows.Count - 1; i++)
{
}

, , i 3 - 1 2, , 0 1, 2. , , .

Answer 5

, for-loop no if-statements:

var table = adapter.GetData(); //get data from repository object -> DataTable
IList<IHouseAnnouncement> list = new List<IHouseAnnouncement>(table.Rows.Count);

for (int i = 0; i < list.Length; i++)
{
   list[i] = new HouseAnnouncement();
   list[i].Area = float.Parse(table.Rows[i][table.areaColumn].ToString());
   list[i].City = table.Rows[i][table.cityColumn].ToString();
}

return list;

, linq-, .:)

Answer 6

Readability, in my opinion, is preferable to be concise with your code - as long as performance is not a victim. In addition, I am sure that anyone who will later need to support the code will also appreciate it.
Even when I maintain my own code, I don’t want to look at it, say a couple of months later, and think, “what the hell am I trying to execute”

Answer 7

I could do something like this:

var table = adapter.GetData(); //get data from repository object -> DataTable

return table.Rows.Take(table.Rows.Count-1).Select(row => new HouseAnnouncement() {
    Area = float.Parse(row[table.powierzchniaColumn].ToString()),
    City = row[table.miastoColumn].ToString()
}).ToList();