La connexion à la base de données sera-t-elle fermée si nous cédons la ligne du lecteur de données et ne lisons pas tous les enregistrements?

15

Tout en comprenant la façon dont les yieldœuvres de mots clés, je suis tombé sur link1 et lien2 sur StackOverflow qui préconise l'utilisation de yield returntout itérer sur le DataReader et il convient à mes besoins aussi bien. Mais cela me fait me demander ce qui se passe, si j'utilise yield returncomme indiqué ci-dessous et si je n'itère pas à travers DataReader entier, la connexion DB restera-t-elle ouverte pour toujours?

IEnumerable<IDataRecord> GetRecords()
{
    SqlConnection myConnection = new SqlConnection(@"...");
    SqlCommand myCommand = new SqlCommand(@"...", myConnection);
    myCommand.CommandType = System.Data.CommandType.Text;
    myConnection.Open();
    myReader = myCommand.ExecuteReader(CommandBehavior.CloseConnection);
    try
       {               
          while (myReader.Read())
          {
            yield return myReader;          
          }
        }
    finally
        {
            myReader.Close();
        }
}


void AnotherMethod()
{
    foreach(var rec in GetRecords())
    {
       i++;
       System.Console.WriteLine(rec.GetString(1));
       if (i == 5)
         break;
  }
}

J'ai essayé le même exemple dans un exemple d'application console et j'ai remarqué lors du débogage que le bloc finalement de GetRecords()n'était pas exécuté. Comment puis-je assurer la fermeture de DB Connection? Existe-t-il un meilleur moyen que d'utiliser un yieldmot-clé? J'essaie de concevoir une classe personnalisée qui sera responsable de l'exécution de certains SQL et procédures stockées sur DB et retournera le résultat. Mais je ne veux pas retourner le DataReader à l'appelant. Je veux également m'assurer que la connexion sera fermée dans tous les scénarios.

Modifier la réponse à la réponse de Ben car il est incorrect de s'attendre à ce que les appelants utilisent correctement la méthode et en ce qui concerne la connexion DB, cela coûtera plus cher si la méthode est appelée plusieurs fois sans raison.

Merci Jakob et Ben pour l'explication détaillée.

GawdePrasad
la source
D'après ce que je vois, vous n'ouvrez pas la connexion en premier lieu
paparazzo
@Frisbee Edited :)
GawdePrasad

Réponses:

12

Oui, vous serez confronté au problème que vous décrivez: jusqu'à ce que vous ayez fini d'itérer le résultat, vous garderez la connexion ouverte. Il y a deux approches générales auxquelles je peux penser pour traiter ceci:

Poussez, ne tirez pas

Actuellement, vous retournez une IEnumerable<IDataRecord>structure de données à partir de laquelle vous pouvez extraire. Au lieu de cela, vous pouvez changer de méthode pour diffuser ses résultats. Le plus simple serait de passer dans un Action<IDataRecord>qui est appelé à chaque itération:

void GetRecords(Action<IDataRecord> callback)
{
    // ...
      while (myReader.Read())
      {
        callback(myReader);
      }
}

Notez que étant donné que vous avez affaire à une collection d'éléments, IObservable/ IObserverpeut être une structure de données légèrement plus appropriée, mais à moins que vous n'en ayez besoin, un simple Actionest beaucoup plus simple.

Évaluer avec enthousiasme

Une alternative consiste simplement à s'assurer que l'itération se termine entièrement avant de revenir.

Vous pouvez normalement le faire en mettant simplement les résultats dans une liste, puis en les renvoyant, mais dans ce cas, il y a la complication supplémentaire de chaque élément étant la même référence pour le lecteur. Vous avez donc besoin de quelque chose pour extraire le résultat dont vous avez besoin du lecteur:

IEnumerable<T> GetRecords<T>(Func<IDataRecord,T> extractor)
{
    // ...
     var result = new List<T>();
     try
     {
       while (myReader.Read())
       {
         result.Add(extractor(myReader));         
       }
     }
     finally
     {
         myReader.Close();
     }
     return result;
}
Ben Aaronson
la source
J'utilise l'approche que vous avez nommée Eagerly Evaluate. Est-ce que ce sera une surcharge de mémoire si mon datareader SQLCommand renvoie plusieurs sorties (comme myReader.NextResult ()) et que je construis une liste de liste? Ou envoyer le datareader à l'appelant [personnellement, je ne le préfère pas] sera très efficace? [mais la connexion sera ouverte plus longtemps]. Ce que je suis précisément confus ici, c'est entre le compromis de garder la connexion ouverte plus longtemps que de créer une liste de liste. Vous pouvez supposer en toute sécurité qu'il y aura près de 500+ personnes visitant ma page Web qui se connecte à DB.
GawdePrasad
1
@GawdePrasad Cela dépend de ce que l'appelant ferait du lecteur. S'il suffit de mettre des articles dans une collection elle-même, il n'y a pas de frais généraux importants. S'il effectuait une opération qui ne nécessitait pas de conserver tout le lot de valeurs en mémoire, il y avait alors une surcharge de mémoire. Cependant, sauf si vous stockez de très grandes quantités, ce ne sont probablement pas des frais généraux dont vous devriez vous soucier. Quoi qu'il en soit, il ne devrait pas y avoir de temps supplémentaire important.
Ben Aaronson
Comment l'approche "pousser, ne pas tirer" résout le problème "jusqu'à ce que vous ayez fini d'itérer sur le résultat, vous garderez la connexion ouverte"? La connexion est toujours ouverte jusqu'à ce que vous ayez fini d'itérer même avec cette approche, n'est-ce pas?
BornToCode
1
@BornToCode Voir la question: "si j'utilise return return comme indiqué ci-dessous et si je n'itère pas à travers DataReader entier, la connexion DB restera-t-elle ouverte pour toujours". Le problème est que vous renvoyez un IEnumerable et chaque appelant qui le gère doit être conscient de ce problème particulier: "Si vous ne finissez pas d'itérer sur moi, je maintiendrai ouverte une connexion". Dans la méthode push, vous enlevez cette responsabilité à l'appelant - il n'a pas la possibilité de répéter partiellement. Au moment où le contrôle leur est rendu, la connexion est fermée.
Ben Aaronson
1
Ceci est une excellente réponse. J'aime l'approche push, car si vous avez une grosse requête que vous présentez de manière paginée, vous pouvez interrompre la lecture plus tôt pour la vitesse en passant un Func <> plutôt qu'une Action <>; cela semble plus propre que de faire paginer la fonction GetRecords.
Whelkaholism
10

Votre finallybloc s'exécutera toujours.

Lorsque vous utilisez yield returnle compilateur, une nouvelle classe imbriquée est créée pour implémenter une machine à états.

Cette classe contiendra tout le code des finallyblocs en tant que méthodes distinctes. Il gardera une trace des finallyblocs qui doivent être exécutés en fonction de l'état. Tous les finallyblocs nécessaires seront exécutés dans la Disposeméthode.

Selon les spécifications du langage C #, foreach (V v in x) embedded-statementest étendu à

{
    E e = ((C)(x)).GetEnumerator();
    try
    {
        while (e.MoveNext())
        {
            V v = (V)(T)e.Current;
            embedded - statement
        }
    }
    finally {
         // Dispose e
    }
}

ainsi l'énumérateur sera supprimé même si vous quittez la boucle avec breakou return.

Pour obtenir plus d'informations sur la mise en œuvre de l'itérateur, vous pouvez lire cet article de Jon Skeet

Éditer

Le problème avec une telle approche est que vous comptez sur les clients de votre classe pour utiliser correctement la méthode. Ils pourraient par exemple obtenir directement l'énumérateur et le parcourir en whileboucle sans le supprimer.

Vous devriez considérer l'une des solutions suggérées par @BenAaronson.

Jakub Lortz
la source
1
C'est extrêmement peu fiable. Par exemple: var records = GetRecords(); var first = records.First(); var count = records.Count()exécutera cette méthode deux fois, ouvrant et fermant la connexion et le lecteur à chaque fois. Le répéter deux fois fait la même chose. Vous pouvez également faire circuler le résultat pendant longtemps avant d'itérer réellement dessus, etc. Rien dans la signature de la méthode n'implique ce genre de comportement
Ben Aaronson
2
@BenAaronson Je me suis concentré sur la question spécifique de l'implémentation actuelle dans ma réponse. Si vous regardez l'ensemble du problème, je suis d'accord, il est préférable d'utiliser la façon dont vous avez suggéré dans votre réponse.
Jakub Lortz
1
@JakubLortz Assez juste
Ben Aaronson
2
@EsbenSkovPedersen Habituellement, lorsque vous essayez de rendre quelque chose à l'épreuve des idiots, vous perdez certaines fonctionnalités. Vous devez équilibrer. Ici, nous ne perdons pas grand-chose en chargeant les résultats avec impatience. Quoi qu'il en soit, le plus gros problème pour moi est la dénomination. Quand je vois une méthode nommée GetRecordsreturn IEnumerable, je m'attends à ce qu'elle charge les enregistrements en mémoire. D'un autre côté, si c'était une propriété Records, je préfère supposer que c'est une sorte d'abstraction sur la base de données.
Jakub Lortz
1
@EsbenSkovPedersen Eh bien, voyez mes commentaires sur la réponse de nvoigt. Je n'ai aucun problème en général avec les méthodes renvoyant un IEnumerable qui fera un travail important une fois itéré, mais dans ce cas, il ne semble pas correspondre aux implications de la signature de la méthode
Ben Aaronson
5

Quel que soit le yieldcomportement spécifique , votre code contient des chemins d'exécution qui ne disposeront pas correctement de vos ressources. Et si votre deuxième ligne lève une exception ou votre troisième? Ou même votre quatrième? Vous aurez besoin d'une chaîne try / finally très complexe, ou vous pouvez utiliser des usingblocs.

IEnumerable<IDataRecord> GetRecords()
{
    using(var connection = new SqlConnection(@"..."))
    {
        connection.Open();

        using(var command = new SqlCommand(@"...", connection);
        {
            using(var reader = command.ExecuteReader())
            {
               while(reader.Read())
               {
                   // your code here.
                   yield return reader;
               }
            }
        }
    }
}

Les gens ont remarqué que ce n'est pas intuitif, que les personnes qui appellent votre méthode peuvent ne pas savoir qu'énumérer le résultat deux fois appellera la méthode deux fois. Eh bien, bonne chance. C'est ainsi que la langue fonctionne. C'est le signal qui IEnumerable<T>envoie. Il y a une raison pour laquelle cela ne revient pas List<T>ou T[]. Les gens qui ne le savent pas doivent être éduqués, pas contournés.

Visual Studio possède une fonctionnalité appelée analyse de code statique. Vous pouvez l'utiliser pour savoir si vous avez correctement supprimé les ressources.

nvoigt
la source
Le langage permet d'itérer IEnumerablepour faire toutes sortes de choses, telles que lever des exceptions totalement indépendantes ou écrire des fichiers de 5 Go sur le disque. Ce n'est pas parce que le langage le permet qu'il est acceptable de renvoyer un IEnumerablequi le fait à partir d'une méthode qui ne donne aucune indication que cela se produira.
Ben Aaronson
1
De retour un IEnumerable<T> est l'indication que ce sera deux fois énumérant deux appels résultat. Vous obtiendrez même des avertissements utiles dans vos outils si vous énumérez plusieurs fois un énumérable. Le point concernant la levée d'exceptions est également valable quel que soit le type de retour. Si cette méthode renvoyait un, List<T>elle lève les mêmes exceptions.
nvoigt
Je comprends votre point de vue et, dans une certaine mesure, je suis d'accord - si vous consommez une méthode qui vous donne un IEnumerableémetteur de mise en garde. Mais je pense aussi qu'en tant que rédacteur de méthode, vous avez la responsabilité d'adhérer à ce que votre signature implique. La méthode est appelée GetRecords, donc quand elle revient, vous vous attendez à ce qu'elle ait, vous savez, obtenu les enregistrements . S'il était appelé GiveMeSomethingICanPullRecordsFrom(ou, de façon plus réaliste GetRecordSourceou similaire), un paresseux IEnumerableserait plus acceptable.
Ben Aaronson
@BenAaronson Je suis d'accord que par exemple dans File, ReadLineset ReadAllLinespeut être facilement distingué et c'est une bonne chose. (Bien que cela soit davantage dû au fait qu'une surcharge de méthode ne peut pas différer uniquement par type de retour). Peut-être que ReadRecords et ReadAllRecords pourraient également convenir ici comme schéma de dénomination.
nvoigt
2

Ce que vous avez fait semble mal, mais fonctionnera bien.

Vous devez utiliser un IEnumerator <> , car il hérite également d' IDisposable .

Mais comme vous utilisez un foreach, le compilateur utilise toujours un IDisposable pour générer un IEnumerator <> pour le foreach.

en fait, le foreach implique beaucoup de choses en interne.

Vérifiez /programming/13459447/do-i-need-to-consider-disposing-of-any-ienumerablet-i-use

Avlin
la source