Conception objet - Constructeur

**Giill** · 05/08/2008, 13h41

Bonjour,

Je fais un petit projet de gestion (très simple) de CDThèque, histoire de me familiariser avec l'objet, que je ne maîtrise pas.

Je me heurte à un petit problème d'objet tout bête...Mais comme je manque d'expérience, je fais appel aux spécialistes.

J'ai créé une classe "Song", qui contient les propriétés et méthodes liées à une chanson :

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
 
private int _songId = "";
private string _songName = "";        
//...
 
public int ID
{
   get { return songId ; }
   set { songId  = value; }
}
public string Name
{
   get { return _songName ; }
   set { _songName  = value; }
}
//...
 
//Constructeur
public Song()
{
}

J'ai aussi créé une classe album, qui contient les propriétés et méthodes liées à un album :

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
 
private int _albumId = "";
private string _albumName = "";  
private Song[] _albumSongs;      
//...
 
public int ID
{
   get { return _albumId ; }
   set { _albumId = value; }
}
public string Name
{
   get { return _albumName ; }
   set { _albumName = value; }
}
public Song[] Songs
{
   get { return _albumSongs; }
   set { _albumSongs= value; }
}
//...
 
//Constructeur
public Album()
{
}
public Album(int aId,string name)
{
      this.ID=aId;      
      this.Name=name;
      GetSongs();      
}
public void GetSongs()
{
      //Ici :
      //Requête qui ramène toutes les chansons de l'album dont l'id est : aId
      //Je mets le résultat de la requête dans une Hashtable lstSongs
     for (int i=0;i<lstSongs.Count;i=i+2)
     {
           this.Songs[i] = new Song();
           this.Songs[i].ID = lstSongs[i];
           this.Songs[i].Name = lstSongs[i +1];
     }
}

Mais je me fais jeter sur la ligne :

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

1
2
 
  this.Songs[i] = new Song();

Avec une erreur du type "La référence d'objet n'est pas définie à une instance d'objet."

Voilà donc le problème.
Je souhaite avoir un constructeur, qui à partir de l'ID de l'album, me charge dans mon objet Album la liste des chansons.
Encore une fois, je ne suis pas à la recherche de la solution pour constituer un outil de gestion de CDThèque idéal, je me sers simplement de cet exemple pour combler mes -grosses!- lacunes en conception objet. Ce qui m'intéresse donc ici, c'est comment faire pour renseigner une "collection" avec des objets en somme...

Merci d'avance pour tout conseil.

**Guulh** · 05/08/2008, 13h53

Songs est un tableau. C'est à dire un objet qui en contient d'autres.

Là, tu as essayé d'appeler une méthode de Songs (écrire machin[i] = bidule équivaut à appeler la méthode this[] de l'objet machin), alors qu'il est encore null. Et en objet, appeler une méthode sur une référence nulle lève une exception (de type NullRefeernceException).

Tu dois donc, avant d'ajouter des éléments à Songs, l'initialiser, avec par exemple :
this.Songs = new Song[10];
si tu supposes que tous tes albums ont 10 chansons.

Après, les tableaux (de la forme T[]) ont une taille fixe ; dans ton cas, tu devrais plutôt te tourner vers une List<Song>, qui est un conteneur de taille variable, auquel il te suffit de faire taListe.Add(taChanson). Il faut aussi bien sûr initialiser une liste avec taListe = new List<Song>().

**Tober** · 05/08/2008, 13h55

Si je ne dis pas de connerie, il faut d'abord instancie ton tableau Songs comme ca :

Code C# :

Sélectionner tout - Visualiser dans une fenêtre à part

Songs = new Song[lataillequetuveux];

Je te conseil aussi de changer ton

Code C# :

Sélectionner tout - Visualiser dans une fenêtre à part

private Song[] _albumSongs;

en

Code C# :

Sélectionner tout - Visualiser dans une fenêtre à part

private IList<Song> _albumSongs;

car c'est plus facile a utiliser, tu peux ajouter des elements autant que tu veux, ...

Edited : Mais euh copieur !

**harz62** · 05/08/2008, 16h42

Utilise un foreach plutôt qu'un for c'est plus simple à utiliser (c'est un peu plus long mais bon on peut pas tout avoir

).

EDIT: par contre ça je suis pas d'accord (erreur de manip peut être ?):

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

private int _albumId = "";

**Giill** · 05/08/2008, 19h48

Merci Guulh et Tober pour la IList ! C'est effectivement très adapté à ce que je veux faire.

Et harz62, bien vu pour l'erreur, il s'agit effectivement d'un copier-coller malheureux...

Pour ceux que ça intéresse donc :

Classe Song :

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
 
private int _songId;
private string _songName;        
//...
 
public int ID
{
   get { return songId ; }
   set { songId  = value; }
}
public string Name
{
   get { return _songName ; }
   set { _songName  = value; }
}
//...
 
//Constructeur
public Song()
{
}

Classe Album :

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
 
private int _albumId;
private string _albumName;  
private System.Collections.Generic.IList<Song> _albumSongs;      
//...
 
public int ID
{
   get { return _albumId ; }
   set { _albumId = value; }
}
public string Name
{
   get { return _albumName ; }
   set { _albumName = value; }
}
public System.Collections.Generic.IList<Song> Songs
{
   get { return _albumSongs; }
   set { _albumSongs= value; }
}
//...
 
//Constructeur
public Album()
{
}
public Album(int aId,string name)
{
      this.ID=aId;      
      this.Name=name;
      GetSongs();      
}
public void GetSongs()
{
      //Ici :
      //Requête qui ramène toutes les chansons de l'album dont l'id est : aId
      //Je mets le résultat de la requête dans une Hashtable lstSongs
      this.Songs = new System.Collections.Generic.List<Song>();
      Song song = new Song();
 
     for (int i=0;i<lstSongs.Count;i=i+2)
     {
           song.ID = lstSongs[i];
           song.Name = lstSongs[i +1];
           this.Songs.Add(song);
           song = new Song();
 
     }
}

Et je note ta remarque harz62 pour le foreach, c'est vrai que c'est une instruction que j'utilise peu...

Merci à tous en tout cas !

**harz62** · 05/08/2008, 20h32

Envoyé par Giill

Et je note ta remarque harz62 pour le foreach, c'est vrai que c'est une instruction que j'utilise peu...

Moi c'est les instructions for que je n'utilise jamais. L'avantage par rapport à un for c'est que l'on ne peut jamais avoir déclencher l'exception IndexoutOfRangeException.

**tomlev** · 05/08/2008, 20h56

Ce bout de code me surprend un peu...

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

1
2
3
4
5
6
7
8
9
10
      Song song = new Song();
 
     for (int i=0;i<lstSongs.Count;i=i+2)
     {
           song.ID = lstSongs[i];
           song.Name = lstSongs[i +1];
           this.Songs.Add(song);
           song = new Song();
 
     }

Evidemment, ça fonctionne, mais ce n'est pas très naturel... en plus en faisant comme ça tu instancies Song une fois de plus que nécessaire (la dernière instance n'est jamais utilisée). Il serait plus naturel, à mon avis, de faire comme ça :

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

1
2
3
4
5
6
7
     for (int i=0;i<lstSongs.Count;i=i+2)
     {
           Song song = new Song();
           song.ID = lstSongs[i];
           song.Name = lstSongs[i +1];
           this.Songs.Add(song);
     }

**Giill** · 05/08/2008, 22h59

Envoyé par harz62

Moi c'est les instructions for que je n'utilise jamais. L'avantage par rapport à un for c'est que l'on ne peut jamais avoir déclencher l'exception IndexoutOfRangeException.

Effectivement, c'est un choix judicieux.
Je me penche là-dessus demain.

Envoyé par tomlev

Evidemment, ça fonctionne, mais ce n'est pas très naturel... en plus en faisant comme ça tu instancies Song une fois de plus que nécessaire (la dernière instance n'est jamais utilisée).

C'est juste pour ta remarque sur l'instance supplémentaire de Song. En fait, je ne sais pas d'où je tiens ça, mais j'ai toujours cru que ce n'était pas une bonne pratique de déclarer une variable dans une boucle (Song song = new Song(); ).
Du coup je la déclare avant la boucle, et l'instancie pour la première fois dans le même temps donc...Ce qui fait que pour ne pas l'instancier deux fois de suite, je choisis de le faire à la fin de la boucle, après le traitement.
Mais comme tu le dis, ça ne change rien, puisqu'une instance "en trop" est créée.

J'avoue ne pas savoir quelle est la bonne pratique dans ce cas, mais je suis ouvert à toute suggestion.

**tomlev** · 05/08/2008, 23h34

Envoyé par Giill

j'ai toujours cru que ce n'était pas une bonne pratique de déclarer une variable dans une boucle

Ben c'est ce que j'ai toujours fait et souvent vu faire... Le seul inconvénient potentiel est un très (très) léger impact sur les performances, puisque l'espace mémoire pour une référence de Song est alloué plusieurs fois au lieu d'une seule (et encore, je ne serais pas surpris que le compilateur détecte ça et optimise de lui-même). Mais bon, à moins que la boucle ne soit exécutée de très nombreuses fois et que les performances soient capitales, je ne pense pas que tu verras une différence.

De toutes façons, si tu préfères tu peux quand-même déclarer la variable en dehors de la boucle, sans pour autant créer une instance "en trop" :

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

1
2
3
4
5
6
7
8
    Song song;
     for (int i=0;i<lstSongs.Count;i=i+2)
     {
           song = new Song();
           song.ID = lstSongs[i];
           song.Name = lstSongs[i +1];
           this.Songs.Add(song);
     }

Rien ne t'oblige à initialiser ta variable aussitôt après sa déclaration

EDIT: En fait j'ai dit une connerie... après examen du code MSIL généré, il semblerait que ça ne change absolument rien : la variable n'est allouée qu'une fois. Donc tu peux déclarer la variable dans la boucle, ça n'a aucun impact sur les perfs (ni sur rien d'autre d'ailleurs, le code généré est identique, à part l'ordre de déclaration des variables dans le MSIL).

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

1
2
3
4
5
6
7
8
        private static void TestLoopDeclare()
        {
            for (int i = 0; i < 100; i++)
            {
                object obj = new object();
                Console.WriteLine(obj);
            }
        }

Code MSIL :

Sélectionner tout - Visualiser dans une fenêtre à part

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
.method private hidebysig static void TestLoopDeclare() cil managed
{
    .maxstack 2
    .locals init (
        [0] int32 i,
        [1] object obj,
        [2] bool CS$4$0000)
    L_0000: nop 
    L_0001: ldc.i4.0 
    L_0002: stloc.0 
    L_0003: br.s L_0018
    L_0005: nop 
    L_0006: newobj instance void [mscorlib]System.Object::.ctor()
    L_000b: stloc.1 
    L_000c: ldloc.1 
    L_000d: call void [mscorlib]System.Console::WriteLine(object)
    L_0012: nop 
    L_0013: nop 
    L_0014: ldloc.0 
    L_0015: ldc.i4.1 
    L_0016: add 
    L_0017: stloc.0 
    L_0018: ldloc.0 
    L_0019: ldc.i4.s 100
    L_001b: clt 
    L_001d: stloc.2 
    L_001e: ldloc.2 
    L_001f: brtrue.s L_0005
    L_0021: ret 
}

**Giill** · 06/08/2008, 08h05

Merci pour ces précisions tomlev.
Où comment réduire en cendres une habitude de programmation vieille de plusieurs années !!!

Je vais donc adopter ta solution :

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

1
2
3
4
5
6
7
8
 
for (int i=0;i<lstSongs.Count;i=i+2)
     {
           Song song = new Song();
           song.ID = lstSongs[i];
           song.Name = lstSongs[i +1];
           this.Songs.Add(song);
     }

**Tober** · 06/08/2008, 09h06

Ce que tu peux faire aussi c'est virer ou modifier ton lstSongs, je trouve "tordu" la facon dont tu stockes les infos... (id0, nom0, id1, nom1, ...)
Pourquoi ne pas utilise un dictionnary par example qui contient comme clef l'id du cd et comme value le nom ?
Ca donnerait un code du style :

Code C# :

Sélectionner tout - Visualiser dans une fenêtre à part

1
2
3
4
5
6
7
8
IDictionary<int, string> listSong = new Dictionary<int, string>();
/* Tu le remplis ici */
 
/* Tu le parcours ici */
foreach (KeyValuePair<int, string> song in listSong)
{
    Songs.Add(new Song(song.Key, song.Value));
}

En meme temps, j'utilise un constructeur pour la classe Song qui prend deux parametres, l'id et le nom.
Donc :

Code C# :

Sélectionner tout - Visualiser dans une fenêtre à part

1
2
3
4
5
6
7
public Song(int id, int nom)
{
    // je change le nom de tes variable car par convention,
    // on ne met pas le nom de la classe dans un attribut de la classe...
    _id = id;
    _name = nom;
}

**alex_noox** · 06/08/2008, 12h00

Bonjour,

Si tu veux en C#, tu peux aussi écrire ta classe Song comme suit :

Code c# :

Sélectionner tout - Visualiser dans une fenêtre à part

1
2
3
4
5
public class Song
{
    public int ID { get; set; }
    public string Name { get; set; }
}

Un seule te sert à déclarer tes attributs et accesseurs

Ensuite tu met les constructeurs dont tu as besoins, mais si le constructeur par défaut ne fait rien pas la peine de le mettre.

**tomlev** · 06/08/2008, 12h17

Envoyé par alex_noox

Si tu veux en C#, tu peux aussi écrire ta classe Song comme suit :

Code c# :

Sélectionner tout - Visualiser dans une fenêtre à part

1
2
3
4
5
public class Song
{
    public int ID { get; set; }
    public string Name { get; set; }
}

Oui, mais seulement à partir de C# 3 (VS2008)

C'est un raccourci pratique, mais avec ça on perd la possibilité de faire des traitements supplémentaires (par exemple lazy loading dans le get, ou notification par évènement dans le set)

**Giill** · 06/08/2008, 14h45

Envoyé par Tober

Ce que tu peux faire aussi c'est virer ou modifier ton lstSongs, je trouve "tordu" la facon dont tu stockes les infos... (id0, nom0, id1, nom1, ...)
Pourquoi ne pas utilise un dictionnary par example qui contient comme clef l'id du cd et comme value le nom ?
Ca donnerait un code du style :

Code C# :

Sélectionner tout - Visualiser dans une fenêtre à part

1
2
3
4
5
6
7
8
IDictionary<int, string> listSong = new Dictionary<int, string>();
/* Tu le remplis ici */
 
/* Tu le parcours ici */
foreach (KeyValuePair<int, string> song in listSong)
{
    Songs.Add(new Song(song.Key, song.Value));
}

Merci pour l'info sur Dictionary. C'est vrai que pour le cas cité, c'est idéal.
Le hic, c'est que c'est limité à deux valeurs (la clé et la valeur). Or, j'ai plus de valeurs ! Là je n'ai mis que l'ID et le Name pour l'exemple, mais derrière j'ai aussi la durée du morceau, etc...Du coup je peux difficilement utiliser cette technique. Cela dit, il existe peut être un Dictionary-like pour plus de 2 valeurs ? Je sais pas si je suis clair...

Envoyé par Tober

En meme temps, j'utilise un constructeur pour la classe Song qui prend deux parametres, l'id et le nom.
Donc :

Code C# :

Sélectionner tout - Visualiser dans une fenêtre à part

1
2
3
4
5
6
7
public Song(int id, int nom)
{
    // je change le nom de tes variable car par convention,
    // on ne met pas le nom de la classe dans un attribut de la classe...
    _id = id;
    _name = nom;
}

C'est noté pour la convention sur les noms de variable, merci pour l'info.

Envoyé par alex_noox

Si tu veux en C#, tu peux aussi écrire ta classe Song comme suit :

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

1
2
3
4
5
6
 
public class Song
{
    public int ID { get; set; }
    public string Name { get; set; }
}

Un seule te sert à déclarer tes attributs et accesseurs
Ensuite tu met les constructeurs dont tu as besoins, mais si le constructeur par défaut ne fait rien pas la peine de le mettre.

C'est noté pour l'écriture plus "compacte", merci pour l'info.
Je vais peut-être rester sur l'écriture "standard" pour le moment, histoire de bien me familiariser avec le tout...Et quand je serais un peu plus "expert" (hum hum), je m'y collerai !

**sorcer1** · 06/08/2008, 15h18

Envoyé par harz62

Moi c'est les instructions for que je n'utilise jamais. L'avantage par rapport à un for c'est que l'on ne peut jamais avoir déclencher l'exception IndexoutOfRangeException.

l'avantage du for par rapport au foreach, c'est qu'on modifier une collection pendant qu'on la parcours !