2010-06-18 8 views
1

J'ai ce segment de code, beaucoup de choses sautée par souci de concision, mais la scène est celle-ci:Quelles sont les options disponibles pour sécuriser ce thread de code?

public class Billing 
    { 
     private List<PrecalculateValue> Values = new List<PrecalculateValue>(); 

     public int GetValue(DateTime date) 
     { 
      var preCalculated = Values.SingleOrDefault(g => g.date == date).value; 
      //if exist in Values, return it 
      if(preCalculated != null) 
      { 
       return preCalculated; 
      } 

      // if it does not exist calculate it and store it in Values 
      int value = GetValueFor(date); 
      Values.Add(new PrecalculateValue{date = date, value = value}); 

      return value; 
     } 

     private object GetValueFor(DateTime date) 
     { 
      //some logic here 
     } 
    } 

J'ai un List<PrecalculateValue> Values où je stocke toutes les valeurs i déjà calculées pour une utilisation ultérieure, je fais ces principalement parce que je ne veux pas recalculer des choses deux fois pour le même client, chaque calcul implique beaucoup d'opérations et prend entre 500 et 1000 ms, et il y a une grande chance de réutiliser cette valeur, à cause d'une certaine récursion dans le trou classe de facturation. Tout cela fonctionne parfaitement jusqu'à ce que j'ai fait un test où j'ai frappé deux calculs simultanés pour deux clients différents, et la ligne Values.Single(g => g.date == date).value a renvoyé une exception car elle a trouvé plus d'un résultat dans la collection. J'ai donc vérifié la liste et stocké les valeurs des deux clients dans la même liste. Que puis-je faire pour éviter ce petit problème?

Répondre

5

Eh bien, tout d'abord cette ligne:

return Values.Single(g => g.date == date).value; 

fait en sorte que les lignes suivantes ne seront jamais appelés. Je suppose que vous avez paraphrasé votre code ici?

Si vous voulez synchroniser les écritures à votre liste Values, la façon la plus simple serait de lock sur un objet commun partout dans le code que vous modifiez la liste:

int value = GetValueFor(date); 

lock (dedicatedLockObject) { 
    Values.Add(new PrecalculateValue{date = date, value = value}); 
} 

return value; 

Mais voici autre chose Il vaut la peine de noter: comme il semble que vous voulez avoir un PrecalculateValue par DateTime, une structure de données plus appropriée serait probablement un Dictionary<DateTime, PrecalculateValue> - il fournira une recherche O rapide (1) basée sur votre clé DateTime, par rapport à un List<PrecalculateValue> qui devrait parcourir pour trouver ce que vous cherchez.

Avec ce changement en place, votre code pourrait ressembler à ceci:

public class Billing 
{ 
    private Dictionary<DateTime, PrecalculateValue> Values = 
     new Dictionary<DateTime, PrecalculateValue>(); 

    private readonly commonLockObject = new object(); 

    public int GetValue(DateTime date) 
    { 
     PrecalculateValue cachedCalculation; 

     // Note: for true thread safety, you need to lock reads as well as 
     // writes, to ensure that a write happening concurrently with a read 
     // does not corrupt state. 
     lock (commonLockObject) { 
      if (Values.TryGetValue(date, out cachedCalculation)) 
       return cachedCalculation.value; 
     } 

     int value = GetValueFor(date); 

     // Here we need to check if the key exists again, just in case another 
     // thread added an item since we last checked. 
     // Also be sure to lock ANYWHERE ELSE you're manipulating 
     // or reading from the collection. 
     lock (commonLockObject) { 
      if (!Values.ContainsKey(date)) 
       Values[date] = new PrecalculateValue{date = date, value = value}; 
     } 

     return value; 
    } 

    private object GetValueFor(DateTime date) 
    { 
     //some logic here 
    } 
} 

Et un dernier conseil: à moins qu'il est essentiel que plus d'une d'une valeur particulière existe dans votre collection, la Single méthode est trop. Si vous préférez obtenir la première valeur sans tenir compte des doublons potentiels, First est à la fois plus sûr (moins de chance d'exception) et plus rapide (car il n'a pas à parcourir toute la collection).

+4

+1 pour l'utilisation d'un dictionnaire codé sur le DateTime. Aussi, si vous utilisez .NET 4, il y a un nouveau ConcurrentDictionary qui gérera le verrouillage pour vous lors de l'ajout de nouveaux objets. –

+0

hehe, yep ceux qui ont commenté si font partie de la logique =), sry à ce sujet. Je vais essayer plus tard – JOBG

+0

ConcurrentDictionary, nice –

1

pourrait utiliser quelque chose comme ça

public int GetValue(DateTime date) 
{ 

    var result = Values.Single(g => g.date == date) ?? GetValueFor(date); 

    lock (Values) 
    { 
     if (!Values.Contains(result)) Values.Add(result); 
    } 
    return result.value; 
} 

private PrecalculateValue GetValueFor(DateTime date) 
{ 
    //logic 
    return new PrecalculateValue() ; 
} 

Conseillerais en utilisant un dictionnaire mais pour une liste de paires de valeurs clés.

Questions connexes