2013-10-11 81 views
0

我有這個循環。 TicketList以109票開始。 nColumns = 100。根據票數來計算我需要的行數。所以在這種情況下,我需要2行。第一行將被填滿,第二行將只有9個條目。我有下面的循環。它只運行NumOfRows一次,並填充前100個,從不循環。For循環 - 不做最後一個循環

我錯過了什麼?

for (int j = 0; j < NumOfRows; j++) 
    { 
     for (int i = 0; i < nColumns; i++) 
     { 
      if (TicketList.Count() > 0) 
      { 
       t = rand.Next(0, TicketList.Count() - 1); 
       numbers[i, j] = TicketList[t]; 
       TicketList.Remove(TicketList[t]); 
      } 
     }     
    } 
+4

如何通過調試器逐步完成?或者添加一些'Trace'語句來看看它在做什麼? –

+0

要清楚,你的外循環只執行一次** ** –

+0

顯示'NumOfRows'的值。附註:最好先洗牌,然後再填充行... –

回答

3

嘗試更改您的代碼以使用更類似於LINQ的功能方法。如果可能會使邏輯更容易。事情是這樣的:

TicketList 
    .OrderBy(x => rand.Next()) 
    .Select((ticket, n) => new 
    { 
     ticket, 
     j = n/NumOfRows, 
     i = n % NumOfRows 
    }) 
    .ToList() 
    .ForEach(x => 
    { 
     numbers[x.i, x.j] = x.ticket; 
    }); 

您可能需要前後翻頁x.i & x.j或使用nColumns代替NumOfRows - 我不知道你的邏輯一直在尋找 - 但是這個代碼可能會更好地工作。

+0

+1。代碼清楚地分離步驟。請注意,'Aggregate()'而不是'ToList.ForEach'序列也可以工作,不需要額外的副本來列出。 –

+0

@AlexeiLevenkov - 'Aggregate'不允許賦值語句。你仍然需要某種循環來允許分配。 – Enigmativity

+0

爲什麼? '.Aggregate(0,(_,x)=>數字[x.i,x.j] = x.ticket);' –

1

除了一些不好的選擇,你的循環似乎很好。我想冒險NumOfRows是不是正確計算。

表達式NumOfRows = (TotalTickets + (Columns - 1))/Columns;應計算正確的行數。

另外,您應該使用屬性版本Count而不是Linq擴展方法,並使用IList<T>.RemoveAt()List<T>.RemoveAt而不是Remove(TicketList[T])

使用Remove()要求枚舉列表以查找要刪除的元素,該列表可能與您定位的索引不同。更不用說,當您已經知道要刪除的正確索引時,您將掃描每次刪除呼叫的列表的50%(平均)。

前面列出的功能方法似乎是矯枉過正。

我試圖複製你的問題,假設使用中的各種變量的某些事實。該循環重複預期的次數。

static void TestMe() 
    { 
     List<object> TicketList = new List<object>(); 

     for (int index = 0; index < 109; index++) 
      TicketList.Add(new object()); 

     var rand = new Random(); 
     int nColumns = 100; 
     int NumOfRows = (TicketList.Count + (nColumns - 1))/nColumns; 
     object[,] numbers; 
     int t; 

     numbers = new object[nColumns, NumOfRows]; 

     for (int j = 0; j < NumOfRows; j++) 
     { 
      Console.WriteLine("OuterLoop"); 
      for (int i = 0; i < nColumns; i++) 
      { 
       if (TicketList.Count > 0) 
       { 
        t = rand.Next(0, TicketList.Count - 1); 
        numbers[i, j] = TicketList[t]; 
        TicketList.RemoveAt(t); 
       } 
      } 
     } 
    } 

您看到的問題必須是您沒有包括在樣品中的問題的結果。

+0

大聲笑。矯枉過正?在我心中優雅簡單。 :-) – Enigmativity

+0

@Enigmativity - 不只是矯枉過正,效率也不高。 – William

+0

在這種情況下,我懷疑它比OP的代碼更有效率。閱讀和調試當然更容易。 – Enigmativity