29 votes

Structure C# IEnumerator/yield potentiellement mauvaise ?

Le contexte : J'ai un tas de chaînes de caractères que je récupère d'une base de données, et je veux les retourner. Traditionnellement, ce serait quelque chose comme ça :

public List<string> GetStuff(string connectionString)
{
    List<string> categoryList = new List<string>();
    using (SqlConnection sqlConnection = new SqlConnection(connectionString))
    {
        string commandText = "GetStuff";
        using (SqlCommand sqlCommand = new SqlCommand(commandText, sqlConnection))
        {
            sqlCommand.CommandType = CommandType.StoredProcedure;

            sqlConnection.Open();
            SqlDataReader sqlDataReader = sqlCommand.ExecuteReader();
            while (sqlDataReader.Read())
            {
                categoryList.Add(sqlDataReader["myImportantColumn"].ToString());
            }
        }
    }
    return categoryList;
}

Mais ensuite, je me dis que le consommateur va vouloir itérer à travers les éléments et ne se soucie pas d'autre chose, et j'aimerais ne pas me limiter à une liste, en soi, donc si je renvoie un IEnumerable, tout est bon/flexible. J'ai donc pensé que je pourrais utiliser une conception de type "yield return" pour gérer cela... quelque chose comme ceci :

public IEnumerable<string> GetStuff(string connectionString)
{
    using (SqlConnection sqlConnection = new SqlConnection(connectionString))
    {
        string commandText = "GetStuff";
        using (SqlCommand sqlCommand = new SqlCommand(commandText, sqlConnection))
        {
            sqlCommand.CommandType = CommandType.StoredProcedure;

            sqlConnection.Open();
            SqlDataReader sqlDataReader = sqlCommand.ExecuteReader();
            while (sqlDataReader.Read())
            {
                yield return sqlDataReader["myImportantColumn"].ToString();
            }
        }
    }
}

Mais maintenant que je lis un peu plus sur le yield (sur des sites comme celui-ci...msdn ne semblait pas le mentionner), c'est apparemment un évaluateur paresseux, qui conserve l'état du populateur, dans l'attente que quelqu'un demande la valeur suivante, et ne l'exécute que jusqu'à ce qu'il retourne la valeur suivante.

Cela semble correct dans la plupart des cas, mais avec un appel à la base de données, cela semble un peu risqué. À titre d'exemple, si quelqu'un demande un IEnumerable que je remplis à partir d'un appel à la base de données, qu'il en parcourt la moitié et qu'il reste coincé dans une boucle... d'après ce que je vois, ma connexion à la base de données va rester ouverte pour toujours.

On dirait qu'il faut s'attendre à des problèmes dans certains cas si l'itérateur ne se termine pas... Est-ce que j'ai raté quelque chose ?

40voto

Jon Skeet Points 692016

C'est une question d'équilibre : voulez-vous forcer toutes les données à être stockées en mémoire immédiatement afin de libérer la connexion, ou voulez-vous profiter de la diffusion en continu des données, au prix de l'immobilisation de la connexion pendant tout ce temps ?

De mon point de vue, cette décision devrait potentiellement revenir à l'appelant, qui sait mieux ce qu'il veut faire. Si vous écrivez le code en utilisant un bloc itérateur, l'appelant peut très a facilement transformé cette forme de flux en une forme entièrement tamponnée :

List<string> stuff = new List<string>(GetStuff(connectionString));

En revanche, si vous vous chargez vous-même de la mise en mémoire tampon, l'appelant n'a aucun moyen de revenir à un modèle de streaming.

Donc j'utiliserais probablement le modèle de flux et je dirais que explicitement dans la documentation ce qu'il fait, et conseiller l'appelant pour qu'il prenne la décision appropriée. Vous pourriez même vouloir fournir une méthode d'aide pour appeler la version streamée et la convertir en liste.

Bien sûr, si vous ne faites pas confiance à vos appelants pour prendre la bonne décision, et si vous avez de bonnes raisons de croire qu'ils ne voudront jamais vraiment diffuser les données (par exemple, elles ne retourneront jamais grand-chose de toute façon), alors optez pour l'approche par liste. Dans tous les cas, documentez-la, car elle pourrait très bien affecter l'utilisation de la valeur de retour.

Une autre option pour traiter de grandes quantités de données consiste à utiliser des lots, bien sûr - cela s'éloigne quelque peu de la question initiale, mais c'est une approche différente à envisager dans une situation où le streaming serait normalement intéressant.

9voto

taoufik Points 2784

Vous n'êtes pas toujours en danger avec l'IEnumerable. Si vous laissez le framework appeler GetEnumerator (ce que la plupart des gens feront), alors vous êtes en sécurité. En fait, vous êtes aussi sûr que la prudence du code qui utilise votre méthode :

class Program
{
    static void Main(string[] args)
    {
        // safe
        var firstOnly = GetList().First();

        // safe
        foreach (var item in GetList())
        {
            if(item == "2")
                break;
        }

        // safe
        using (var enumerator = GetList().GetEnumerator())
        {
            for (int i = 0; i < 2; i++)
            {
                enumerator.MoveNext();
            }
        }

        // unsafe
        var enumerator2 = GetList().GetEnumerator();

        for (int i = 0; i < 2; i++)
        {
            enumerator2.MoveNext();
        }
    }

    static IEnumerable<string> GetList()
    {
        using (new Test())
        {
            yield return "1";
            yield return "2";
            yield return "3";
        }
    }

}

class Test : IDisposable
{
    public void Dispose()
    {
        Console.WriteLine("dispose called");
    }
}

La possibilité de laisser la connexion à la base de données ouverte ou non dépend également de votre architecture. Si l'appelant participe à une transaction (et que votre connexion est enrôlée automatiquement), la connexion sera de toute façon maintenue ouverte par le framework.

Un autre avantage de yield est (lors de l'utilisation d'un curseur côté serveur), votre code ne doit pas lire toutes les données (exemple : 1 000 articles) de la base de données, si votre consommateur veut sortir de la boucle plus tôt (exemple : après le 10e article). Cela peut accélérer l'interrogation des données. En particulier dans un environnement Oracle, où les curseurs côté serveur sont le moyen le plus courant de récupérer des données.

8voto

Richard Szalay Points 42486

Vous ne manquez rien. Votre exemple montre comment NE PAS utiliser le retour de rendement. Ajoutez les éléments à une liste, fermez la connexion et renvoyez la liste. La signature de votre méthode peut toujours retourner IEnumerable.

Edit : Cela dit, Jon n'a pas tort (quelle surprise !) : il y a de rares occasions où la diffusion en continu est en fait la meilleure chose à faire du point de vue des performances. Après tout, s'il s'agit de 100 000 (1 000 000 ? 10 000 000 ?) lignes, vous n'avez pas envie de tout charger en mémoire en premier.

5voto

Marc Gravell Points 482669

Soit dit en passant, notez que le IEnumerable<T> l'approche est essentiellement ce que les fournisseurs LINQ (LINQ-to-SQL, LINQ-to-Entities) font pour vivre. Cette approche présente des avantages, comme le dit Jon. Cependant, elle présente aussi des problèmes certains, notamment (pour moi) en termes de (combinaison de) séparation et d'abstraction.

Ce que je veux dire ici, c'est que :

  • dans un scénario MVC (par exemple), vous voulez que votre étape "get data" soit obtenir réellement des données afin que vous puissiez tester le fonctionnement de l'appareil sur le site de l'entreprise. contrôleur et non le voir (sans avoir à se rappeler d'appeler .ToList() etc)
  • vous ne pouvez pas garantir qu'une autre implémentation de DAL sera capable pour diffuser des données en continu (par exemple, un appel POX/WSE/SOAP ne peut généralement pas diffuser des enregistrements en continu) ; et vous ne voulez pas nécessairement que le comportement soit confusément différent (c'est-à-dire que la connexion reste ouverte pendant l'itération avec une implémentation, et fermée pour une autre)

Cela rejoint un peu mes réflexions ici : LINQ pragmatique .

Mais je dois insister sur le fait qu'il y a des moments où le streaming est hautement souhaitable. Ce n'est pas une simple question de "toujours ou jamais"...

2voto

Une façon un peu plus concise de forcer l'évaluation de l'itérateur :

using System.Linq;

//...

var stuff = GetStuff(connectionString).ToList();

Prograide.com

Prograide est une communauté de développeurs qui cherche à élargir la connaissance de la programmation au-delà de l'anglais.
Pour cela nous avons les plus grands doutes résolus en français et vous pouvez aussi poser vos propres questions ou résoudre celles des autres.

Powered by:

X