Code Anecdote: optimiser le code pour plusieurs boucles imbriquées

Question

Je suis tombé sur ce code aujourd'hui et je me demande quelles sont les façons de l'optimiser.

De toute évidence, le modèle est difficile à changer car il est hérité, mais intéressé à obtenir des opinions.

Modifié certains noms et brouillé une certaine logique de base à protéger. En faire un wiki pour que les amateurs de code puissent répondre sans se soucier des points.

Answer 1

Cela dépend beaucoup des données. Vous devez établir un profil avec un ensemble de données «typique», identifier les goulots d'étranglement, puis envisager des optimisations appropriées en fonction des données de votre profil.

Answer 2

if (testPayment.Customer.Name != customer.Name){continue;} 

Cela ne devrait pas être nécessaire pour commencer - il est certain que tous les paiements effectués pour une commande donnée concernent le même client?

Je n'aime pas du tout cette fonction, si je réussis à passer un payment_id, alors je ne m'attendrais qu'à recevoir soit ce paiement spécifique, soit null ... Rien de tout cela ne cherche ...

me semble que vous devez penser à redessiner beaucoup de code, et je pense qu'il va bien au-delà de cette fonction spécifique ...

Answer 3

Si vous pouvez ajouter des membres à la classe Payment, ajoutez un didRefund booléen qui est défini sur true chaque fois que la chaîne RefundPayment est définie sur "refund". Cela vous permet d'éviter la chaîne compare.

Avant la boucle si vous n'initialisez finalPayment comme ceci:

finalPayment = new Payment; 
finalPayment.Value = -1.0e12 

vous pouvez éviter le test nul dans la boucle. (En supposant qu'aucun des clients n'effectue de paiements négatifs d'un milliard de dollars)

Answer 4

Le premier test (if (payment.RefundPayment == null)) est redondant. Le deuxième test utilisant String.Compare fonctionne avec des chaînes null. Vous pouvez utiliser cette "optimisation" dans la deuxième place, vous utilisez cette comparaison dans la boucle foreach.

Answer 5

Vous pouvez déplacer l'une des conditions dans une fonction qui lui est propre car elle est répétée deux fois (test pour elle et son contraire). Vous pouvez également mettre les conditions ensemble et envelopper la boucle entière dans le premier conditionnel. De cette façon, vous n'avez qu'un point de sortie pour la fonction. Si cela ne semble pas gérable, vous pouvez envelopper le foreach-loop dans une fonction et l'appeler ainsi.

private static boolean IsRefundPayment(Payment payment) { 
    return payment.RefundPayment != null && String.Compare(payment.RefundPayment, "refund", true) == 0; 
} 

private static Payment FindPayment(Order order, Customer customer, int paymentId) { 
    Payment payment = Order.Payments.FindById(paymentId); 
    if (payment == null || IsRefundPayment(payment)) { 
     foreach (Payment testpayment in Order.payments) { 
      if (testPayment.Customer.Name == customer.Name && !testPayment.Cancelled && !IsRefundPayment(payment) 
       && (testPayment.Value > payment.Value)) { 
       payment = testPayment; 
      } 
     } 
    } 
    return payment; 
}