2010-04-06 4 views
3

의이 코드를 살펴 보자 : 덜 코드에서이 쓸 수있는 더 좋은 방법이 더 나은 패션 (:-)를해야합니다)더 나은 방법으로 이것을 쓰는 방법?

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; 

인가? 어쩌면 람다를 사용하여 (하지만 어떻게 알려주 는가?)

미리 감사드립니다.

답변

9

목록에 새로운 HouseAnnouncement을 추가하지 마십시오. 루프는 마지막 행인 행에는 실행되지 않지만, 실제 코드가 아닌 예제에서 오류가 있다고 가정합니다. 나는 보통 간결함을 통해 가독성을 위해 이동

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

하지만이 꽤 읽을 같은 느낌 :

은 당신이 뭔가를 할 수 있습니다.

람다에서 여전히 DataTable을 캐시하고 table.powierzchniaColumn을 사용할 수 있지만 실제로는 필요하지 않은 클로저를 사용하지 않도록 제거했습니다 (클로저는 람다의 내부 구현에 상당한 복잡성을 초래합니다. , 그래서 가능하다면 나는 그들을 피한다.) 이 컴파일러가 생성하는 실제 IL에 복잡성을 추가합니다

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(); 
} 

하지만을 수행해야합니다 그들이 당신이 열 참조를 유지하는 것이 중요 경우

, 당신은 이런 식으로 작업을 수행 할 수 있습니다 너를 속인다.

+0

이것은 winnar입니다. 또한, 가능한 경우 IEnumerable을 반환하는 메서드를 얻을 수 있는지 확인한 다음 원하는 경우 소비자가 목록에 캐스팅하도록 할 수 있습니다. 람다 반환 값을 IHouseAnnouncement에 캐스팅하거나'as'를 사용하여 IEnumerable 를 반환하도록 할 수 있습니다. –

+0

"목록으로 변환"= "목록으로 변환" –

+0

방법 : Rows.Cast ()은 실행되지 않고 컴파일되지 않습니다. – Dariusz

0

가독성은 성능이 희생자가 아닌 한 코드와 간결하게하는 것이 바람직합니다. 또한, 나중에 코드를 유지해야하는 사람이라면 누구나 그것을 고맙게 생각할 것입니다.
내 코드를 관리 할 때도 2 개월 후에 말하고 싶지 않고 "도대체 내가 무엇을 성취하려고했는지"라고 생각합니다.

2

"if 문"이 필요하지 않습니다. "for loop"는 이미이 경우를 처리합니다.

또한 "for 루프"는 테이블 행의 수가 1 일 때 실행되지 않습니다. 이는 의도적으로 설계된 것이 아니며 잘못된 것일 수 있습니다. 당신이이 문제를 해결하려면, 그냥 걸릴 "-1"

1

음, 한 가지, 당신은 off-by-one 오류가 나타납니다

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

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

당신의 테이블에 3 개의 행이있는 경우 i이 3 - 1보다 작거나 2인데, 이는 행 0과 행 1에서는 실행되지만 행 2에서는 실행되지 않음을 의미합니다. 이는 의도 한 것과 다를 수 있습니다.

0

나는 같은 것을 할 수 있습니다

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(); 
5

당신은 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(); 
+0

이것은 가장 간결하고 표현력이 뛰어나고 우아한 솔루션입니다. –

+0

불행히도 컴파일되지 않습니다. 'DataTable.Rows'는'IEnumerable '을 구현하지 않으므로, 당신은 그것을 호출하기 위해'캐스트 ()'을 호출해야합니다. 그럼에도 불구하고 나는 그것이 더 간결 해지는 것을 보지 못한다. (두 개가 아닌 세 문장이기 때문이다.) –

+0

또한 데이터를 두 번 검색하고 있습니다. –

1

훨씬 간단 갈 수 에 대한 루프 하나없는 경우 명령문이 :

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-version보다 많은 문자가 필요하지만 프로그래머의 두뇌에 의해 더 빨리 구문 분석됩니다. :)