Est-il possible de passer des DataReaders à des constructeurs ?

Question

Je maintiens un code C# 2.0 et le programmeur utilise un modèle de lecture d'une collection d'objets commerciaux en ouvrant un DataReader et en le passant ensuite au constructeur de l'objet. Je ne vois rien d'anormal à cela, mais j'ai l'impression que ce n'est pas bien. Est-ce que c'est acceptable ?

private static void GetObjects()
{
    List<MyObject> objects = new List<MyObject>();
    string sql = "Select ...";
    SqlConnection connection = GetConnection();
    SqlCommand command = new SqlCommand(sql, connection);
    connection.Open();
    SqlDataReader reader = command.ExecuteReader(CommandBehavior.CloseConnection);
    while (reader.Read())
        objects.Add(new MyObject(reader));
    reader.Close();
}

public MyObject(SqlDataReader reader)
{
    field0 = reader.GetString(0);
    field1 = reader.GetString(1);
    field2 = reader.GetString(2);
}

Answer 1

En passant le DataReader au constructeur de l'objet, vous établissez un lien très étroit entre l'objet métier et votre choix de technologie de persistance.

Au minimum, ce couplage étroit rendra la réutilisation et les tests difficiles ; au pire, il pourrait faire en sorte que les objets métier en sachent beaucoup trop sur la base de données.

La résolution de ce problème n'est pas très difficile : il suffit de déplacer l'initialisation de l'objet hors du constructeur et de la placer dans une classe d'usine distincte.

Answer 2

Faire circuler un lecteur me gêne, à moins qu'il ne s'agisse d'une méthode d'aide non publique pour gérer la copie d'une ligne. Mais certainement pas à un constructeur.

Le passage d'un lecteur connecte votre constructeur (vous n'avez pas placé MyObject dans une classe mais vous appelez new MyObject() ) dans votre stockage de données et je suppose que votre objet n'est pas écrit pour cela ?

Si c'était moi :

private static void GetObjects()
{
    List<MyObject> objects = new List<MyObject>();
    string sql = "Select ...";
    using (SqlConnection connection = GetConnection())
    {
        SqlCommand command = new SqlCommand(sql, connection);
        connection.Open();
        using(SqlDataReader reader = command.ExecuteReader(CommandBehavior.CloseConnection);)
        {
            while (reader.Read())
                objects.Add(_ReadRow(reader));
        }
    }
}

private static MyObject _ReadRow(SqlDataReader reader)
{
    MyObject o = new MyObject();
    o.field0 = reader.GetString(0);
    o.field1 = reader.GetString(1);
    o.field2 = reader.GetString(2);

    // Do other manipulation to object before returning

    return o;
}

class MyObject{}

Answer 3

Je ne procéderais pas de cette manière, mais je ne vois pas de problème majeur.

Answer 4

(EDIT : Cette réponse se concentre uniquement sur "quelles sont les implications à un niveau relativement bas" plutôt que sur les implications globales de la conception. Il semble que d'autres réponses aient couvert ces aspects, je ne ferai donc pas de commentaire :)

Il y a certainement quelque chose qui ne va pas dans le code que vous avez donné, car rien ne ferme la connexion, la commande ou le lecteur. Plus précisément, votre ligne d'affectation de connexion devrait normalement ressembler à ceci :

using (SqlConnection connection = GetConnection())
{
    ...
}

Vous pouvez penser que ce n'est que du pinaillage, que ce n'est qu'un exemple de code et que le nettoyage n'a pas d'importance, mais la "propriété" des ressources qui sont utilisées dans le cadre de l'application de la directive est un élément essentiel de la politique de l'UE en matière de ressources. besoin Le nettoyage est précisément le problème que pose l'adoption d'une DataReader à un constructeur.

Je pense qu'il n'y a pas de problème tant que l'on sait à qui appartient le lecteur par la suite. Par exemple, dans Image.FromStream Dans certains cas, l'image est propriétaire du flux par la suite et peut ne pas apprécier que vous le fermiez vous-même (en fonction du format de l'image et de quelques autres facteurs). Dans d'autres cas, c'est toujours à vous qu'il incombe de fermer le flux. Cela doit être très soigneusement documenté, et si le type avec le constructeur prend la propriété, il doit implémenter IDisposable pour faciliter le nettoyage et pour qu'il soit plus évident qu'un nettoyage est nécessaire.

Dans votre cas, il semble que le constructeur soit no s'approprier le lecteur, ce qui est une alternative tout à fait valable (et plus simple). Il suffit de documenter cela, et l'appelant devra toujours fermer le lecteur de manière appropriée.

Answer 5

Je ferais passer l'entité dans un IDataReader car cela facilite le test de MyObject.