Discussion :

[User Name]

Bonjour a tous,

Je suis en train de m'entrainer pour mon rattrapage de C++ en refaisant l'epreuve que j'ai ratee. J'ai un peu de mal avec quelques notions, exemple:

Voici mon code:

Pion.cxx

Code :

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

1
2
3
4
5
6
7
8
9
10
11
#include "Pion.h"
 
Pion::Pion(std::string coul, Position& p)
{
	type="pion";
	couleur=coul;
	symbole="P";
	valeur=1;
	Position* ptemp = new Position(p.getX(), p.getY());
	pos = ptemp;
}

Pion.h

Code :

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

1
2
3
4
5
6
7
8
9
10
11
12
#ifndef PION
#define PION
 
#include "Piece.h"
#include "Position.h"
 
class Pion : Piece
{
	public:
		Pion(std::string, Position&); //Couleur, Position
};
#endif

Position.cxx

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
51
52
53
54
55
56
57
#include "Position.h"
#include "stdio.h"
 
Position::Position(int X, int Y)
{
	x=X;
	y=Y;
}
 
Position::Position(const Position& P)
{
	x=P.x;
	y=P.y;
}
 
void Position::setX(int X)
{
	x=X;
}
 
void Position::setY(int Y)
{
	y=Y;
}
 
int Position::getX()
{
	return x;
}
 
int Position::getY()
{
	return y;
}
 
void Position::AffichePosition()
{
	std::cout<<"X="<<x<<" et Y="<<y;
}
 
std::ostream& operator << (std::ostream& ini, const Position& p)
{
	ini<<"X="<<p.x<<" et Y="<<p.y;
	return ini;
}
 
bool operator == (const Position &q, const Position& p)
{
	if ((p.x == q.x) && (p.y == q.y))
	{
		return true;
	}
	else
	{
		return false;
	}
}

Position.h

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
#ifndef POSITION
#define POSITION
 
#include <iostream>
 
class Position
{
	private:
	int x;
	int y;
 
	public:
	Position(int=0, int=0);
	Position(const Position&);
 
	int getX();
	int getY();
	void setX(int);
	void setY(int);
 
	void AffichePosition();
	friend std::ostream& operator << (std::ostream&, const Position&);
	friend bool operator == (const Position&, const Position&);
 
};
 
#endif

Pion herite de Piece, une classe abstraite, mais je ne pense pas que vous en ayez besoin pour me repondre.

Le compilateur me dit:

Code :

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

1
2
3
Pion.cxx: In constructor ‘Pion::Pion(std::string, Position&)’:
Pion.cxx:10:8: error: invalid conversion from ‘Position*’ to ‘int’
Pion.cxx:10:8: error:   initializing argument 1 of ‘Position::Position(int, int)’

et je refuse de le croire, pour la simple et bonne raison que p.getX() retourne bel et bien un int et non un Position*, comme vous pouvez le constater.

Si quelqu'un peut m'expliquer et me donner la solution...

Merci d'avance.

[dragonjoker59]

Si je ne me trompe pas, là il râle sur l'assignation de 'pos'. C'est une variable membre, je suppose, mais de quel type ? Si ce n'est pas une variable membre, d'où ça sort ?

[User Name]

pos est un attribut de la classe mere de Pion, qui s'appelle Piece. Cet attribut est protected, et de type Position.

[User Name]

J'ai trouve, il fallait mettre ca:

Code :

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

1
2
3
 
Position* ptemp = new Position(p.getX(), p.getY());
pos = *ptemp;

C'est vraiment au pif que j'ai trouve, ca n'a aucun sens pour moi de faire comme ca...
D'abord, comment se fait-il que le new renvoie un pointeur sur structure?
Je croyais que ca creait une structure en bonne et due forme...

[dragonjoker59]

Je crois qu'il faut que tu revoies les cours de C/C++. En Java ton assertion serait vraie, mais pas en C, ni C++.
http://chgi.developpez.com/pointeur/

[valAa]

Hello,

J'ai trouve, il fallait mettre ca:

Code :

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

1
2
3
 
Position* ptemp = new Position(p.getX(), p.getY());
pos = *ptemp;

Je ne vois pas l'intérêt de ce code.
Ici tu crées un pointeur local dans ton constructeur (ptemp), puis tu copies sa valeur dans ton attribut pos. À la sortie du constructeur, le pointeur ptemp n'existe plus, mais la mémoire n'a pas été libérée.
Tu as donc une fuite de mémoire.

Pour initialiser ton attribut pos sans passer par un pointeur qui ne sert à rien ici :

Code :

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

1
2
 
pos = Position(p.getX(), p.getY())

Autre chose, un constructeur devrait de préférence utiliser une liste d'initialisation pour initialiser ses membres, plutôt que de faire des affectations dans son corps.

[3DArchi]

Salut,
Ce devrait pas être quelque chose comme ça :

Code :

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

1
2
Pion::Pion(std::string const &coul, Position const & p):Piece(coul,p)
{}

Code :

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

1
2
3
4
5
6
7
8
Piece::Piece(std::string const&coul, Position const&p)
:
        couleur(coul)
	,type("pion")
	,symbole("P")
	,valeur(1)
	,pos(p)
{}

Je pense que ton allocation est une erreur et génère une fuite mémoire.

[User Name]

Je ne suis malheureusement pas assez avancé en C++ pour connaître ce type de syntaxes (on l'a vue en coup de vent en cours, mais on ne s'en est jamais servis, et on n'a plus les notes du cours correspondant).

Ca fait quoi

Code :

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

Pion::Pion(std::string const &coul, Position const & p):Piece(coul,p)

?

PS: Je suis éminemment allergique aux références, surtout constantes. J'ai toujours un problème incompréhensible avec (du style "discards qualifiers" selon que je me rappelle), alors je m'en sers le moins possible (je sais que c'est très mal, mais je suis dans l'urgence).

[3DArchi]

Salut,

Je ne suis malheureusement pas assez avancé en C++ pour connaître ce type de syntaxes (on l'a vue en coup de vent en cours, mais on ne s'en est jamais servis, et on n'a plus les notes du cours correspondant).

T'as un plan B pour ton rattrapage ?

Ca fait quoi

Code :

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

Pion::Pion(std::string const &coul, Position const & p):Piece(coul,p)

?

Ca appelle le constructeur de la classe de base avec les paramètres adéquats. J'ai supposé qu'il s'agissait de cela vu qu'aucun des éléments initialisés dans le constructeur de Piece n'était défini dans cette classe. J'ai donc (abusivement?) conclu qu'il s'agissait de membre de la classe de base.

PS: Je suis éminemment allergique aux références, surtout constantes. J'ai toujours un problème incompréhensible avec (du style "discards qualifiers" selon que je me rappelle), alors je m'en sers le moins possible (je sais que c'est très mal, mais je suis dans l'urgence).

Les bonnes pratiques et les bases (comme les références constantes) permettent de ne pas faire n'importe quoi lorsqu'on est en mode panique

[User Name]

Oui mais le souci c'est que quand j'utilise des références constantes, je ne peux pas appeler les accesseurs de l'objet dont j'utilise une référence constante. Il me signale que si je fais appel à ses accesseurs (getters), je le modifie (ce qui évidemment est faux, puisque je n'accède qu'en lecture à l'objet).

N'ayant jamais résolu ce problème, j'ai décidé de ne plus m'en servir. Et je ne peux dans la plupart des cas pas mettre const à la fin de la fonction dans laquelle j'utilise la référence, parce qu'il s'agit bien souvent... d'un constructeur.

[Joel F]

c'est tes getter qui doivent etre const.

Faut revoir les baes la, tu n'arrivera a rien en faisant n'imp.

[User Name]

Je vais m'y remettre. En attendant explique-moi stp pourquoi ce code ne fonctionne pas:

Echiquier.h

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
 
#ifndef ECHIQUIER
#define ECHIQUIER
 
#include <vector>
#include "Piece.h"
 
class Echiquier
{
	private:
		std::vector<Piece*> pieces;
 
	public:
		void ajouter(Piece*);
		void afficher();
		int ValeurDuJoueur(std::string);
		Piece& saisirPiece (const Position&) const;
};
 
#endif

Echiquier.cxx

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
 
int Echiquier::ValeurDuJoueur(std::string couleur)
{
	int compteur=0;
	for (int j=0; j<pieces.size(); j++)
	{
		if (pieces.at(j)!=NULL && pieces.at(j)->getCouleur()==couleur) //Lever eventuellement exception si couleur non valide
		{
			compteur+=pieces.at(j)->getValeur();
		}
	}
	return compteur;	
}
 
Piece& Echiquier::saisirPiece (const Position& p) const
{
	int i=0;
	while (i<pieces.size() && (pieces.at(i)->getPosition().getX()!=p.getX() || pieces.at(i)->getPosition().getY()!=p.getY()))
	{
		i++;
	}
	if (i<pieces.size())
	{
		//std::cout<<"La piece qui se trouve en ("<<p.getX()<<","<<p.getY()<<") est "<<pieces.at(i)->toDebug()<<std::endl;
		return pieces.at(i);
	}
	else
	{
		//std::cout<<"Cette piece n'est pas sur l'echiquier."<<std::endl;
		return NULL;
	}
}

Avec une erreur de compilation sibyllinne (orthographe?):

Code :

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

1
2
3
4
5
 
Echiquier.cxx:49:72: error: passing ‘const Position’ as ‘this’ argument of ‘int Position::getX()’ discards qualifiers
Echiquier.cxx:49:120: error: passing ‘const Position’ as ‘this’ argument of ‘int Position::getY()’ discards qualifiers
Echiquier.cxx:56:21: error: invalid initialization of reference of type ‘Piece&’ from expression of type ‘Piece* const’
Echiquier.cxx:61:10: error: invalid initialization of non-const reference of type ‘Piece&’ from an rvalue of type ‘int’

Alors que celui-ci fonctionne:

Echiquier.h

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
 
#ifndef ECHIQUIER
#define ECHIQUIER
 
#include <vector>
#include "Piece.h"
 
class Echiquier
{
	private:
		std::vector<Piece*> pieces;
 
	public:
		void ajouter(Piece*);
		void afficher();
		int ValeurDuJoueur(std::string);
		Piece* saisirPiece (Position);
};
 
#endif

Echiquier.cxx

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
 
Piece* Echiquier::saisirPiece (Position p)
{
	int i=0;
	while (i<pieces.size() && (pieces.at(i)->getPosition().getX()!=p.getX() || pieces.at(i)->getPosition().getY()!=p.getY()))
	{
		i++;
	}
	if (i<pieces.size())
	{
		//std::cout<<"La piece qui se trouve en ("<<p.getX()<<","<<p.getY()<<") est "<<pieces.at(i)->toDebug()<<std::endl;
		return pieces.at(i);
	}
	else
	{
		//std::cout<<"Cette piece n'est pas sur l'echiquier."<<std::endl;
		return NULL;
	}
}

[JolyLoic]

Code :

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

1
2
 
Echiquier.cxx:49:72: error: passing ‘const Position’ as ‘this’ argument of ‘int Position::getX()’ discards qualifiers

Ce qu'il te dit, en clair, c'est :

L'objet sur lequel j'essaye d'appeler la fonction getX (as ‘this’ argument of ‘int Position::getX()’) est un objet constant (passing ‘const Position’) mais la fonction getX n'est pas une fonction constante (discards qualifiers, sous entendu le qualifier 'const').

Bien entendu, tu ne montres pas le code de Position, mais je suis prêt à parier qu'il ressemble à :

Code :

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

1
2
3
4
5
6
7
8
9
10
11
12
class Position
{
public:
  int getX()
private:
  int myX;
};
 
int Position::getX()
{
  return myX;
}

Alors qu'il devrait ressembler à :

Code :

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

1
2
3
4
5
6
7
8
9
10
11
12
class Position
{
public:
  int getX() const;
private:
  int myX;
};
 
int Position::getX() const
{
  return myX;
}

Ou une alternative, plus complexe, mais permettant plus de choses :

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
class Position
{
public:
  int &getX();
  int getX() const;
private:
  int myX;
};
 
int Position::getX() const
{
  return myX;
}
 
int &Position::getX() 
{
  return myX;
}

[User Name]

Ok, merci. Je vais optimiser mon code en ce sens.

J'en profite pour une autre question, pour ne pas avoir a reposter mon code dans un autre topic. Je suis en train d'apprendre par moi-meme les iterateurs.

Dans cette partie du code:

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
void Cavalier::deplacements()
{
	cases.push_back(new Position(pos.getX()-2,pos.getY()-1));
	cases.push_back(new Position(pos.getX()-2,pos.getY()+1));
	cases.push_back(new Position(pos.getX()-1,pos.getY()-2));
	cases.push_back(new Position(pos.getX()-1,pos.getY()+2));
	cases.push_back(new Position(pos.getX()+1,pos.getY()-2));
	cases.push_back(new Position(pos.getX()+1,pos.getY()+2));
	cases.push_back(new Position(pos.getX()+2,pos.getY()-1));
	cases.push_back(new Position(pos.getX()+2,pos.getY()+1));
 
	//Pas besoin si PositionException
	for (std::vector<Position*>::iterator i=cases.begin(); i!=cases.end()-1; i++)
	{
		if ((*i)->getX()<0 || (*i)->getY()<0 || (*i)->getX()>7 || (*i)->getY()>7)
		{
			cases.erase(i);
		}
	}
}

J'ai un Segmentation Fault au niveau du erase. En fait, je voudrais supprimer toutes les positions creees ci-dessus qui sont en dehors de la grille 8x8. Mais je ne sais pas pourquoi j'ai un Segmentation Fault...

Si j'enleve juste case.erase(i), j'obtiens ceci comme positions:

Code :

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

1
2
Deplacements possibles du Cavalier Noir en (7,4): 8
(5,3), (5,5), (6,2), (6,6), (8,2), (8,6), (9,3), (9,5),

[User Name]

J'ai cru comprendre que quand on efface un membre, le vecteur retrecit, mais la valeur de cases.end() dans la boucle for ne se met pas a jour. Donc j'ai remplace par ca:

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
	std::vector<Position*>::iterator i=cases.begin();
	do
	{
		//std::cout<<"Salut"<<std::endl;
		if ((*i)->getX()<0 || (*i)->getY()<0 || (*i)->getX()>7 || (*i)->getY()>7)
		{
			cases.erase(i);
			i=cases.begin();
		}
		else
		{
			i++;
		}
	}
	while (i!=cases.end());

Ca marche, mais je ne sais pas si c'est tres fiable comme code...

[screetch]

end() est bien mis a jour, mais pas "i" dans ton code
et ca va crasher sur "i++"
pour que ca marche il faut plutot un truc comme ca:

Code :

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

1
2
3
4
5
6
7
8
9
10
11
12
	//Pas besoin si PositionException
	for (std::vector<Position*>::iterator i=cases.begin(); i!=cases.end()-1; /*nothing*/)
	{
		if ((*i)->getX()<0 || (*i)->getY()<0 || (*i)->getX()>7 || (*i)->getY()>7)
		{
			i = cases.erase(i);
		}
		else
		{
			i++;
		}
	}

[JolyLoic]

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
	std::vector<Position*>::iterator i=cases.begin();
	do
	{
		//std::cout<<"Salut"<<std::endl;
		if ((*i)->getX()<0 || (*i)->getY()<0 || (*i)->getX()>7 || (*i)->getY()>7)
		{
			cases.erase(i);
			i=cases.begin();
		}
		else
		{
			i++;
		}
	}
	while (i!=cases.end());

Ca marche, mais je ne sais pas si c'est tres fiable comme code...

Dans cette version de ton code, à chaque effacement, tu repars au début. Ça n'est pas efficace du tout (imagine, tu as 1 000 000 éléments dans ton tableau, et les 1 000 derniers sont à enlever, avec cet algorithme, tu vas parcourir et tester environ 1 000 000 000 cases...). En outre, dans un vecteur, chaque erase doit décaler toutes les cases qui viennent derrière. Donc si cette fois ce sont les 1 000 premiers éléments qui ne sont pas bons, tu vas avoir 1 000 000 000 décalages à faire environ.

La solution de screetch n'a pas le premier problème, mais a le second. Le problème c'est qu'écrire la bonne solution en terme de performances demande d'utiliser des écritures d'une certaine complexité. Je te propose donc de te contenter pour l'instant de la solution de screetch, et si tu as des soucis de performances soit de passer à une liste au lieu d'un vecteur, soit d'étudier la méthode plus complexe (http://cpp.developpez.com/faq/cpp/?p...ssion_elements) soit de filtrer les mauvais éléments avant de les mettre dans le vecteur.


Ton code a aussi un autre problème. Appeler erase va détruire une case de ton vecteur. Ton vecteur contient des pointeurs. Il va donc détruire un pointeur. Par contre, la valeur pointée n'est pas détruite, elle. Ton code est donc plein de fuites mémoire.

Là aussi, plusieurs possibilités s'offrent à toi :
- Faire delete manuellement pour détruire les valeurs pointées avant chaque erase (et à d'autres endroits dans le code, selon toute probabilité). C'est lourd et source d'erreurs.
- Au lieu de pointeurs, utiliser un truc qui ressemble à des pointeurs, mais se charge de faire delete sur la valeur pointée au moment où le "pointeur" est détruit. Ça se nomme un pointeur intelligent, mais là aussi, je pense qu'il te faut mieux maîtriser les bases pour bien comprendre comment ça marche.
- Utiliser un vector<Position>. C'est simple, ça marche sans rien faire de plus (là quand on détruit une case, on détruit une position, c'est exactement ce qu'on voulait faire), ça probablement plus performant (plus besoin de faire des dizaines de new, qui sont des opérations coûteuses). Plus rapide, plus simple, plus sûr, on croit rêver (c'est souvent un défaut de personnes ayant été formée en Java ou C# de mettre en C++ plein de pointeurs là où ce n'est pas nécessaire. Est-ce ton cas ?)