擺脫嵌套循環的問題

Question

我有問題正確地擺脫這些嵌套循環。代碼試圖做的是指示客戶租用了某個電影。將電影和客戶都與arraylist對象的屬性進行比較，然後如果全部檢出電影對象的name屬性和ID屬性作爲字符串添加到另一個arraylist。只要我使用第一部電影（來自電影）和第一位顧客（來自顧客），但是如果我嘗試將其他電影進一步向其他顧客下放到我的數組列表中，則它會將租借的電影添加到customerRentedMovies數組列表中，但打印出「其他消息」。我想我需要擺脫foreach（blabla）循環嗎？或可以轉到使用？評論移除（看上去有點凌亂，可以進一步解釋，如果需要的話）

public void RentMovie(string titel, int movieID, string name, int customerID) 
    { 
     foreach (Customer customer in customers) 
     { 
      if (name == customer.Name && customerID == customer.CustomerID) 
      { 
       foreach (MovieInfo movie in movies) 
       { 
        if (titel == movie.Titel && movieID == movie.MovieID) 
        { 
         movie.rented = true; 
         string rentedMovie = string.Format("{0} ID: {1}", movie.Titel, movie.MovieID); 
         customer.customerRentedMovies.Add(rentedMovie); 

         break; 
        } 

        else { Console.WriteLine("No movie with that titel and ID!"); } 

       } 
      break;  
      } 
      else { Console.WriteLine("No customer with that ID and name"); } 
     } 

    } 

Answer 1

你的方法違反了單一職責原則 - 每個類或方法應該有且只有改變的一個責任和原因。您有負責做3件不同的事情了一個方法：

• 找到一個客戶
• 找到一個電影
• 租用電影給客戶。

這使得

• 難以測試
• 難以維持
• 很難理解

這是一個Code Smell。

你應該重構你的方法是這樣的，委託尋找客戶，尋找電影以自己的方法的責任：如果你沒有使用LINQ的FindCustomer()和FindMovie方法可能

public void RentMovie(string titel , int movieID , string name , int customerID) 
{ 
    Customer customer = FindCustomer(customerID , name) ; 
    MovieInfo movie = FindMovie(movieID , titel) ; 

    if (customer == null) 
    { 
    Console.WriteLine("No customer with that ID and name"); 
    } 

    if (movie == null) 
    { 
    Console.WriteLine("No movie with that titel and ID!") ; 
    } 

    if (customer != null && movie != null) 
    { 
    string rentedMovie = string.Format("{0} ID: {1}" , movie.Titel , movie.MovieID); 
    movie.rented = true; 
    customer.customerRentedMovies.Add(rentedMovie); 
    } 

    return ; 
} 

是這樣的：

private MovieInfo FindMovie(int movieID , string titel) 
{ 
    MovieInfo instance = null ; 
    foreach(MovieInfo movie in movies) 
    { 
    if (movie.MovieID == movieID && movie.Titel == titel) 
    { 
     instance = movie ; 
     break ; 
    } 
    } 
    return instance ; 
} 

private Customer FindCustomer(int customerID , string name) 
{ 
    Customer instance = null ; 
    foreach(Customer customer in customers) 
    { 
    if (customer.CustomerID == customerID && customer.Name == name) 
    { 
     instance = customer ; 
     break ; 
    } 
    } 
    return instance ; 
} 

如果你使用凌，同樣的方法可能是：

private MovieInfo FindMovie(int movieID , string titel) 
{ 
    return movies.Where(x => x.MovieID == movieID && x.Titel == titel).SingleOrDefault() ; 
} 

private Customer FindCustomer(int customerID , string name) 
{ 
    return customers.Where(x => x.Name == name && x.CustomerID == customerID).SingleOrDefault() ; 
} 

代碼現在更簡單，更容易理解和自我描述。當需要進行更改時，更改也會變得更容易。

Answer 2

如果你想打破這兩個循環，只需用return。

foreach (Customer customer in customers) 
{ 
    if (name == customer.Name && customerID == customer.CustomerID) 
    { 
     foreach (MovieInfo movie in movies) 
     { 
      if (titel == movie.Titel && movieID == movie.MovieID) 
      { 
       movie.rented = true; 
       string rentedMovie = string.Format("{0} ID: {1}", movie.Titel, movie.MovieID); 
       customer.customerRentedMovies.Add(rentedMovie); 

       return; //break out of both loops 
      } 
      else { Console.WriteLine("No movie with that titel and ID!"); }  
     }       
    }     
    else { Console.WriteLine("No customer with that ID and name"); } 
} 

Answer 3

既然你做你的循環後，沒有什麼可以只調用return;，你有你的休息。

Answer 4

我感到那實際上你不需要嵌套循環的話 - 你不改變基於customermovies反正。另外，我會使用LINQ。所以：

var customer = customers.FirstOrDefault(c => c.Name == customerName && 
              c.CustomerId == customerId); 
if (customer == null) 
{ 
    Console.WriteLine("No customer with that ID and name"); 
    return; 
} 

var movie = movies.FirstOrDefault(m => m.Name == movieName && 
            m.MovieId == movieId); 

if (movie == null) 
{ 
    Console.WriteLine("No movie with that ID and name"); 
    return; 
} 

movie.rented = true; 
string rentedMovie = string.Format("{0} ID: {1}", movie.Titel, movie.MovieID); 
customer.customerRentedMovies.Add(rentedMovie); 

（我可能會實際上改變什麼返回或拋出一個異常，如果客戶或電影不能被發現，但這是另一回事）

重要的一點是，現在沒有明確的循環 - 我們說我們試圖找到聲明並採取相應的行動。同樣，沒有嵌套，分開了兩個問題（尋找電影和尋找客戶）。我們現在可以輕鬆地將每個部分提取到單獨的方法中 - 特別是如果我們使用異常而不是記錄和返回。它會然後是：

Customer customer = FindCustomer(customerId, customerName); 
Movie movie = FindMovie(movieId, movieName); 

movie.rented = true; 
string rentedMovie = string.Format("{0} ID: {1}", movie.Titel, movie.MovieID); 
customer.customerRentedMovies.Add(rentedMovie); 

更簡單。

Answer 5

如果你能使用LINQ，你的代碼可以變得更清潔：

var customer = 
    customers.FirstOrDefault(x => x.Name == name && x.CustomerID == customerID); 
if (customer == null) { // Error } 

var movie = movies.FirstOrDefault(x => x.Title == title && x.MovieID == movieID); 
if (movie == null) { // Error } 

// Rental logic here 

而且，不應ID是足以唯一標識的對象？

Answer 6

如果您只想在「for」循環結束時顯示錯誤，則需要設置某種標誌，表示您找不到電影或其他東西。我已經改變了你的代碼：

public void RentMovie(string titel, int movieID, string name, int customerID) 
    { 
     bool hasCustomer, hasMovie; 

     hasCustomer = false; 
     hasMovie = false; 

     foreach (Customer customer in customers) 
     { 
      if (name == customer.Name && customerID == customer.CustomerID) 
      { 
       hasCustomer = true; 
       foreach (MovieInfo movie in movies) 
       { 
        if (titel == movie.Titel && movieID == movie.MovieID) 
        { 
         hasMovie = true; 
         movie.rented = true; 
         string rentedMovie = string.Format("{0} ID: {1}", movie.Titel, movie.MovieID); 
         customer.customerRentedMovies.Add(rentedMovie); 

         break; 
        } 

       } 
       if (hasMovie == false) { 
        Console.WriteLine("No movie with that titel and ID!"); 
       } 
      break;  
      } 
     } 

     if (hasCustomer == false) { 
      Console.WriteLine("No customer with that ID and name"); 
     } 
    } 

Answer 7

第一的foreach中的if/else是會得到打爲每一位顧客，如果你有25個客戶，然後它會擊中else語句的10倍找到正確的前客戶（假設正確的客戶是第11個之一）你爲什麼不使用LINQ獲取和設置值

Customer customer = customers.FirstOrDefault(x => x.Name = name && x.CustomerID = customerID); 

    if(customer != null) //will be null if customer isn't found 
    { 
     //DO the same thing here for the movies and once you find the 
     //movie set the properties and add to the collection 
    }