éviter fuites mémoires

Version imprimable

Bonjour,

Voici ma classe Ball dans ball.h :

Code:

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
class Ball
{
	public:
		Ball(int id, int radius, int x, int y, float velocity_x, float velocity_y);
 
		~Ball();
 
		void move_(int l_left, int l_top, int l_right, int l_bot); // les limites gauche, haut, droite, bas 
		void collision(Ball** balls);
 
                const static int s_num_balls = 10;
                static bool s_collision;
                static int s_num_collisions;
		static int s_min_velocity;
		static int s_max_velocity;
 
        int m_id;
		int m_radius;
		int m_x;
		int m_y;
		double m_velocity_x;
		double m_velocity_y;
		double m_accel_x;
		double m_accel_y;
 
    private:
 
        void free();
};

Je crée dynamiquement des instances de classe ainsi dans main.cpp :

Code:

1
2
3
for(int i=0; i<Ball::s_num_balls; i++){
      balls[i] = new Ball(...);
}

Voici le destructeur vide de ma classe Ball :

Code:

1
2
3
4
Ball::~Ball() //destructeur
{
	//delete this;
}

Et voici la fonction close() dans main.cpp

Code:

1
2
3
4
5
6
7
8
void close()
{
    for(int i=0; i<Ball::s_num_balls; i++){
        delete balls[i];
    }
 
    ...
}

Est-ce que je m'y prends bien pour libérer la mémoire occupée par chaque objet Ball lors de sa déstruction ?

Merci

Je ne vois rien de choquant mais pourquoi tu ne crées pas une classe Ball_Util :koi: :koi: (<- nom douteux)

Je me demande même si delete[] n'appelle pas tous les destructeurs respectifs :koi: :koi:

Code:

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
class Ball_Util {
public:
 
    Ball_Util() : num_balls(0) { /* ... */ }
 
    ~Ball_Util() {
//  Version if array_balls is a C-array
        for(int nb=0; nb < num_balls; nb++){
            delete array_balls[nb];
            array_balls[nb] = NULL;
        }
    }
 
    void add_ball(Ball& ) { /* ... */ }
    void add_multi_balls(vector<Ball>& ) { /* ... */ }
 
//  void operator+ (Ball& ) { /* ... */ } // Or a non-member version
 
    void test_collisions() { /* ... */ }
 
private: /*protetected:*/
 
    Ball* array_balls;
//  vector<Ball> array_balls;
 
private: /*protetected:*/
 
    int num_balls;
    bool collision;
    int num_collisions;
    int min_velocity;
    int max_velocity;
};

Édit: Médinoc a raison :mrgreen:, mais c'est du code vite-fait pour montrer qu'on peut concevoir de manière à éviter 2-3 trucs comme des variables statiques dans chaque balle.

08/07/2014, 10h31
Médinoc

La classe, je l'appellerais probablement ball_collection ou bien ball_manager.

Citation:

Envoyé par foetus

Code:

void add_ball(Ball& ) { /* ... */ }

Ta fonction add_ball(), si elle prend une référence en paramètre, elle a intérêt à faire une copie de l'objet, et non en prendre possession.

@guitz: Ta fonction membre collision() devrait retourner un Booléen plutôt que jouer avec une variable globale. De plus, elle devrait probablement être déclarée const, ainsi que son paramètre, si la détection de collision n'est pas censée modifier les balles.
08/07/2014, 14h56
r0d

Problème: éviter fuites mémoires. (titre du fil)
Solution: Ne pas utiliser de pointeur.

Hope it helps.
08/07/2014, 16h24
ternel

C'est vrai que les pointeurs nus sont la source première de fuites mémoires.
S'en passer autant que possible au profit des références et du RAII réduit considérablement le risque.

pour rappel, le RAII de pointeur est std::unique_ptr et std::share_ptr.
08/07/2014, 18h09
Ehonn

Pourquoi tu n'utilises pas std::vector dans ton code ?

Grosso modo, pour ne pas avoir de fuites mémoires, il suffit de ne pas utiliser directement l'allocation dynamique.
11/07/2014, 13h16
guitz

Merci à tous pour vos réponses.

Je vais créer la classe ball_manager, merci je n'y avais pas pensé.

Citation:

Envoyé par foetus

...c'est du code vite-fait pour montrer qu'on peut concevoir de manière à éviter 2-3 trucs comme des variables statiques dans chaque balle.

Je crois que le principe de la variable statique c'est qu'elle n'est pas propre à chaque instance de classe, mais propre à la classe elle-même

Citation:

Envoyé par foetus

Ball_manager() : num_balls(0) { /* ... */ }

Que signifie num_balls(0) stp ?

Citation:

Envoyé par Medinoc

@guitz: Ta fonction membre collision() devrait retourner un Booléen plutôt que jouer avec une variable globale. De plus, elle devrait probablement être déclarée const, ainsi que son paramètre, si la détection de collision n'est pas censée modifier les balles.

Ok merci du conseil

Pour les autres, je crée d'abord ma classe Ball_manager, je fais en sorte d'avoir un code plus propre, et ensuite je prends en compte vos conseils pour éviter les fuites mémoires
11/07/2014, 14h19
Médinoc

Citation:

Envoyé par guitz

Que signifie num_balls(0) stp ?

Une liste d'initialisation est une syntaxe spécifique aux constructeurs d'une classe. Elle permet d'appeler explicitement les constructeurs des classes mères et des variables membres (et en C++11, appeler un autre constructeur de la même classe).

Dans le cas présenté, cela initialise explicitement la variable membre num_balls à zéro.

Merci Médinoc.

Bon j'ai terminé de nettoyer mon code suivant vos conseils :

Ball_manager.h :

Code:

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
class Ball_manager{
public:
 
    Ball_manager(std::vector<Ball*> array_balls, std::vector<SDL_Rect> spriteBallsClips, int num_balls, int min_velocity, int max_velocity);
 
    ~Ball_manager();
 
    void add_ball(std::vector<Ball*> array_balls);
    void add_multi_balls(std::vector<Ball*> array_balls);
    bool collision(Ball* ball1, std::vector<Ball*> array_balls);
 
    static int s_num_collisions;
    static int s_num_balls;
 
    std::vector<Ball*> m_array_balls;
    std::vector<SDL_Rect> m_spriteBallsClips;
 
    bool m_collision;
    int m_min_velocity;
    int m_max_velocity;
};

Ball_manager.cpp :

Code:

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 "Ball_manager.h";
 
int Ball_manager::s_num_collisions = 0;
int Ball_manager::s_num_balls = 0;
 
Ball_manager::Ball_manager(std::vector<Ball*> array_balls, std::vector<SDL_Rect> spriteBallsClips , int num_balls, int min_velocity, int max_velocity){
 
    m_array_balls = array_balls;
    m_spriteBallsClips = spriteBallsClips;
    s_num_balls = num_balls;
    m_min_velocity = min_velocity;
    m_max_velocity = max_velocity;
 
}
 
Ball_manager::~Ball_manager() {
//  Version if array_balls is a C-array
    for(int nb=0; nb < Ball_manager::s_num_balls; nb++){
            delete m_array_balls[nb];
            m_array_balls[nb] = NULL;
        }
}
 
 
void Ball_manager::add_ball(std::vector<Ball*> array_balls) {
 
}
 
void Ball_manager::add_multi_balls(std::vector<Ball*> array_balls) {
 
}
 
bool Ball_manager::collision(Ball* ball1, std::vector<Ball*> array_balls){
 
    int compteur_collisions = 0;
 
    if(ball1->m_id != Ball_manager::s_num_balls - 1){
        for(int i=ball1->m_id+1; i<Ball_manager::s_num_balls; i++){
            // si la distance entre 2 balles est plus petite que la somme des 2 rayons respectifs des 2 balles, alors il y a collision
            if(hitTest(ball1, array_balls[i])){
 
                updateVelocities(ball1, array_balls[i]);
                updatePositions(ball1, array_balls[i]);
 
                compteur_collisions++;
                Ball_manager::s_num_collisions++;
            }
        }
    }
 
    if(compteur_collisions > 0){// si cette balle est entrée en collision avec au moins une autre balle...
        return true;
    }else{
        return false;
    }
 
}

Ball.h :

Code:

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
class Ball
{
	public:
		//Initializes variables
		Ball(int id, int radius, int x, int y, float velocity_x, float velocity_y);
 
		//Deallocates memory
		~Ball();
		//Gets image dimensions
 
		void move_(int l_left, int l_top, int l_right, int l_bot); // les limites gauche, haut, droite, bas
 
        int m_id;
		int m_radius;
		int m_x;
		int m_y;
		double m_velocity_x;
		double m_velocity_y;
		double m_accel_x;
		double m_accel_y;
 
    private:
 
        void free();
};
 
int hitTest(Ball* a, Ball* b); // teste si 2 balles se chevauchent
//bool placeBallsOnTangeant(Ball* a, Ball* b);
void updatePositions(Ball* a, Ball* b);
void updateVelocities(Ball* a, Ball* b);
int rand_a_b(int a, int b, int c); // génère un entier aléatoire entre a (inclus) et b (exclus)

Ball.cpp :

Code:

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
58
59
60
61
62
63
64
65
66
67
68
69
70
71
72
73
74
75
76
77
78
79
80
81
82
83
84
85
86
87
88
89
90
91
92
93
94
95
96
97
98
99
100
101
102
103
104
105
106
107
108
109
110
111
Ball::Ball(int id, int radius, int x, int y, float velocity_x, float velocity_y) //constructeur
{
    m_id = id;
	m_radius = radius;
	m_x = x;
	m_y = y;
	m_velocity_x = velocity_x;
	m_velocity_y = velocity_y;
	m_accel_x = 0.0;
	m_accel_y = 0.0;
}
 
void Ball::move_(int l_left, int l_top, int l_right, int l_bot){
    if(m_x> l_right){
        m_x = l_right;
        m_velocity_x *= -1;
    }
    if(m_x< l_left){
        m_x = l_left;
        m_velocity_x *= -1;
    }
    if(m_y + m_velocity_y > l_bot){
        m_y = l_bot;
        m_velocity_y *= -1;
    }
    if(m_y + m_velocity_y < l_top){
        m_y = l_top;
        m_velocity_y *= -1;
    }
 
    m_x += m_velocity_x;
    m_y += m_velocity_y;
}
 
int rand_a_b(int a, int b, int c){
    srand(time(NULL) + c);
    return rand()%(b-a) +a;
}
 
int hitTest(Ball* a, Ball* b){
 
    double balls_distance_x = static_cast<double>(a->m_x - b->m_x);
    double balls_distance_y = static_cast<double>(a->m_y - b->m_y);
    return balls_distance_x*balls_distance_x + balls_distance_y*balls_distance_y < pow(static_cast<double>(a->m_radius + b->m_radius), 2.0);
}
 
void updatePositions(Ball* a, Ball* b){//une fois hitTest détecté on place les balles qui se chevauchent, en bordure de tangeante au point de collision
 
    double collision_angle, angle_pos_a, angle_pos_b, a_norm_pos, b_norm_pos, a_ccs_x, a_ccs_y, b_ccs_x, b_ccs_y;
 
    collision_angle = atan2(static_cast<double>(a->m_y - b->m_y), static_cast<double>(a->m_x - b->m_x));
 
    angle_pos_a = atan2(static_cast<double>(a->m_y), static_cast<double>(a->m_x));
    angle_pos_b = atan2(static_cast<double>(b->m_y), static_cast<double>(b->m_x));
    a_norm_pos = sqrt(static_cast<double>(a->m_x*a->m_x + a->m_y*a->m_y));
    b_norm_pos = sqrt(static_cast<double>(b->m_x*b->m_x + b->m_y*b->m_y));
 
    a_ccs_x = a_norm_pos*cos(angle_pos_a - collision_angle);
    a_ccs_y = a_norm_pos*sin(angle_pos_a - collision_angle);
    b_ccs_x = b_norm_pos*cos(angle_pos_b - collision_angle);
    b_ccs_y = b_norm_pos*sin(angle_pos_b - collision_angle);
 
    if(a_ccs_x > b_ccs_x){
        a_ccs_x = (a_ccs_x + b_ccs_x)/2.0 + a->m_radius; // on positionne les boules de façon tangeante au point de collision dans le nouveau système de coordonnées
        b_ccs_x = a_ccs_x - a->m_radius - b->m_radius;
    }else{
        a_ccs_x = (a_ccs_x + b_ccs_x)/2.0 - a->m_radius; // on positionne les boules de façon tangeante au point de collision dans le nouveau système de coordonnées
        b_ccs_x = a_ccs_x + a->m_radius + b->m_radius;
    }
 
    a->m_x = cos(collision_angle)*a_ccs_x - sin(collision_angle)*a_ccs_y; // On rote les vecteurs position dans le bon système de coordonnées en utilisant une matrice de rotation 2D
    a->m_y = sin(collision_angle)*a_ccs_x + cos(collision_angle)*a_ccs_y;
    b->m_x = cos(collision_angle)*b_ccs_x - sin(collision_angle)*b_ccs_y;
    b->m_y = sin(collision_angle)*b_ccs_x + cos(collision_angle)*b_ccs_y;
 
}
 
void updateVelocities(Ball* a, Ball* b){ //une fois hitTest détecté on on réinitialise les vecteurs vitesses respectifs des 2 balles
 
    double collision_angle, angle_velocity_a, angle_velocity_b, a_norm_velocity, b_norm_velocity, a_ccs_velocity_x, a_ccs_velocity_y, b_ccs_velocity_x, b_ccs_velocity_y;
    double a_new_ccs_velocity_x, b_new_ccs_velocity_x, a_new_ccs_norm_velocity, b_new_ccs_norm_velocity, a_mass, b_mass;
 
    collision_angle = atan2(static_cast<double>(a->m_y - b->m_y), static_cast<double>(a->m_x - b->m_x));
    angle_velocity_a = atan2(static_cast<double>(a->m_velocity_y), static_cast<double>(a->m_velocity_x));
    angle_velocity_b = atan2(static_cast<double>(b->m_velocity_y), static_cast<double>(b->m_velocity_x));
 
    a_norm_velocity = sqrt(static_cast<double>(a->m_velocity_x*a->m_velocity_x + a->m_velocity_y*a->m_velocity_y));
    b_norm_velocity = sqrt(static_cast<double>(b->m_velocity_x*b->m_velocity_x + b->m_velocity_y*b->m_velocity_y));
 
    a_ccs_velocity_x = a_norm_velocity*cos(angle_velocity_a - collision_angle); //ccs = changed coordinate system. la tangeante au point de collision devient perpendiculaire à l'axe des absisses, ce qui simplifie le calcul
    a_ccs_velocity_y = a_norm_velocity*sin(angle_velocity_a - collision_angle);
    b_ccs_velocity_x = b_norm_velocity*cos(angle_velocity_b - collision_angle);
    b_ccs_velocity_y = b_norm_velocity*sin(angle_velocity_b - collision_angle);
 
    a_mass = (static_cast<double>(4/3)*3.141592*powf(static_cast<float>(a->m_radius), 3.0))/50.0;
    b_mass = (static_cast<double>(4/3)*3.141592*powf(static_cast<float>(b->m_radius), 3.0))/50.0;
    a_new_ccs_velocity_x = (a_ccs_velocity_x*(a_mass - b_mass) + 2*b_mass*b_ccs_velocity_x)/(a_mass + b_mass);// on peut se contenter de faire à présent une collision 1D sur l'axe des x
    b_new_ccs_velocity_x = (b_ccs_velocity_x*(a_mass - b_mass) + 2*b_mass*a_ccs_velocity_x)/(a_mass + b_mass);
 
    a->m_velocity_x = cos(collision_angle)*a_new_ccs_velocity_x - sin(collision_angle)*a_ccs_velocity_y; // On rote les vecteurs vitesse dans le bon système de coordonnées en utilisant une matrice de rotation 2D
    a->m_velocity_y = sin(collision_angle)*a_new_ccs_velocity_x + cos(collision_angle)*a_ccs_velocity_y;
    b->m_velocity_x = cos(collision_angle)*b_new_ccs_velocity_x - sin(collision_angle)*b_ccs_velocity_y;
    b->m_velocity_y = sin(collision_angle)*b_new_ccs_velocity_x + cos(collision_angle)*b_ccs_velocity_y;
 
    //Ball::s_collision = true;
}
 
Ball::~Ball() //destructeur
{
	//delete this;
}

main.cpp :

Code:

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
58
59
60
61
62
63
64
65
66
67
68
69
70
71
72
73
74
75
76
77
78
79
80
81
82
83
84
85
86
87
88
89
90
91
92
93
94
95
96
97
98
99
100
101
102
103
104
105
106
107
108
109
110
111
112
113
114
115
116
117
118
119
120
121
122
123
124
125
126
127
128
129
130
131
132
133
134
135
136
137
138
139
140
141
...
 
const int SCREEN_WIDTH = 1400;
const int SCREEN_HEIGHT = 900;
 
char texte[15]; //utile pour convertir Balls::num_collisions en char, pour le rendu de texte dans le menu
 
int rand_velocity_init_x, rand_velocity_init_y; //utilisé lors de la création d'instance de balles (vecteurs vitesse initiaux)
int max_num_balls_per_row = (SCREEN_WIDTH/100)/2;
int compteur_collisions = 0;
 
std::vector<Ball*> array_balls;
std::vector<SDL_Rect> array_spriteBallsClips;
Ball_manager* ball_manager1 = new Ball_manager(array_balls, array_spriteBallsClips, 10, 1, 20);
//Ball* balls[ball_manager1->m_num_balls] = {NULL};
Button* buttons[Button::s_num] = {NULL};
...
SDL_Window* gWindow = NULL;
SDL_Renderer* gRenderer = NULL;
SDL_Event e;
 
LTexture gNumCollisionsTexteTexture, gButtonSpriteSheetTexture, gSpriteSheetBallsTexture, gBackground;
 
SDL_Rect gSpriteButtonsClips[Button::s_num];
 
...
 
bool loadMedia()
{
	...
 
	    SDL_Rect rect;
 
        //Set balls sprites
		for(int i=0; i<Ball_manager::s_num_balls; i++){
 
            rect.w = 100;
            rect.h = 100;
            if(i%4==0){
                rect.x =   0;
                rect.y =   0;
            }else if(i%4==1){
                rect.x = 100;
                rect.y = 100;
            }else if(i%4==2){
               rect.x = 0;
               rect.y = 100;
            }else{
                rect.x = 100;
                rect.y = 0;
            }
 
            ball_manager1->m_spriteBallsClips.push_back(rect);
 
            ...
 
            ball_manager1->m_array_balls.push_back(new Ball(i, 50, 100 + (i % max_num_balls_per_row) * (100*2), 100 + (i / max_num_balls_per_row) * (100*2), rand_velocity_init_x, rand_velocity_init_y));
		}
		...
 
	}
 
	return success;
}
 
void close()
{
 
    delete ball_manager1;
    ball_manager1 = NULL;
 
    for(int i=0; i<Button::s_num; i++){
        delete buttons[i];
    }
 
	//Free loaded images
	gBackground.free();
	gSpriteSheetBallsTexture.free();
	gButtonSpriteSheetTexture.free();
	gNumCollisionsTexteTexture.free();
 
	//Free global font
	TTF_CloseFont( gFont );
	gFont = NULL;
 
	//Destroy window
	SDL_DestroyRenderer( gRenderer );
	SDL_DestroyWindow( gWindow );
	gWindow = NULL;
	gRenderer = NULL;
 
	//Quit SDL subsystems
	TTF_Quit();
	IMG_Quit();
	SDL_Quit();
}
 
 
int main( int argc, char* args[] )
{
    ...
			while( !quit )
			{
				//Handle events on queue
				while( SDL_PollEvent( &e ) != 0 )
				{
					//User requests quit
					if( e.type == SDL_QUIT )
					{
						quit = true;
					}
				}
 
                     //Clear screen
                SDL_RenderClear( gRenderer );
                gBackground.render(0, 0, gRenderer);
 
                for(int i=0; i<Ball_manager::s_num_balls; i++){
 
                    //switch
                    ball_manager1->m_array_balls[i]->move_(200, 100, SCREEN_WIDTH - 2*ball_manager1->m_array_balls[i]->m_radius, SCREEN_HEIGHT - 2*ball_manager1->m_array_balls[i]->m_radius);
 
                    compteur_collisions += ball_manager1->collision(ball_manager1->m_array_balls[i], ball_manager1->m_array_balls); //appel de la méthode collision
 
                    ...
 
                    gSpriteSheetBallsTexture.render( ball_manager1->m_array_balls[i]->m_x, ball_manager1->m_array_balls[i]->m_y, gRenderer, &ball_manager1->m_spriteBallsClips[i] );
 
                }
 
				//Update screen
				SDL_RenderPresent( gRenderer );
			}
		}
	}
 
	//Free resources and close SDL
	close();
 
	return 0;
}

Voilà pouvez-vous me dire svp si dans les grandes lignes ça vous semble correct ?

C'est probablement très bien pour du C-- :E

Voici quelques remarques rapides.

Ball_manager.h

Code:

Ball_manager(std::vector<Ball*> array_balls, std::vector<SDL_Rect> spriteBallsClips, int num_balls, int min_velocity, int max_velocity);

- Ne pas utiliser de pointeur nu. Tu as std::unqiue_ptr, std::shared_ptr et std::reference_wrapper pour ça. http://en.cppreference.com/w/
- Il manque des const :
- - Si tu as besoin de modifier un objet, tu le passes par référence : T &.
- - Si tu n'as pas besoin de le modifer, tu le passes par référence sur membre constant : T const &, sauf pour les types de base car une copie n'est pas "coûteuse".
- - Les fois où tu fais une copie sont rares : lorsque tu as besoin de modifier la copie passée.
(Ces remarques sont valables dans le reste du code)

Code:

static int s_num_collisions;

- Cet attribut (et les autres aussi) doit être privés car l'utilisateur de la classe n'as pas de raison de le modifier (faudra faire un accesseur si cela a du sens)

Ball_manager.cpp
Code:

1 2 3 4 5 6 7 8 9 Ball_manager::Ball_manager(std::vector<Ball*> array_balls, std::vector<SDL_Rect> spriteBallsClips , int num_balls, int min_velocity, int max_velocity){ m_array_balls = array_balls; m_spriteBallsClips = spriteBallsClips; s_num_balls = num_balls; m_min_velocity = min_velocity; m_max_velocity = max_velocity; }
- Utilise la liste d'initialisation : FAQ C++ - Mes constructeurs doivent-ils utiliser les listes d'initialisation ou l'affectation ?.
Code:

1 2 3 4 5 6 7 Ball_manager::~Ball_manager() { // Version if array_balls is a C-array for(int nb=0; nb < Ball_manager::s_num_balls; nb++){ delete m_array_balls[nb]; m_array_balls[nb] = NULL; } }
- La responsabilité de la mémoire n'est pas très clair (c'est un des avantages de std::unqiue_ptr, std::shared_ptr et std::reference_wrapper).
- Généralement, c'est celui qui alloue qui supprime, alors que ici tu as un transfère non évident de responsabilité.
- Pour avoir la taille d'un tableau (std::vector), c'est ton_vecteur.size().
- En C++11, on utilise nullptr.
Code:

1 2 3 4 5 if(compteur_collisions > 0){// si cette balle est entrée en collision avec au moins une autre balle... return true; }else{ return false; }
- return compteur_collisions > 0;.

Ball.h
- Identation
- Utilise des références [sur membre constant], pas des pointeurs.

Code:

~Ball();

- Inutile : Si le destructeur en fait rien, autant ne pas l'écrire, le compilateur le générera pour toi.

Code:

int rand_a_b(int a, int b, int c); // génère un entier aléatoire entre a (inclus) et b (exclus)

- Cette fonction n'a rien a faire dans ce fichier

Ball.cpp
Code:

1 2 3 4 int rand_a_b(int a, int b, int c){ srand(time(NULL) + c); return rand()%(b-a) +a; }
- En C++11, utilise #include <chrono> (plus simple).
Code:

1 2 3 4 Ball::~Ball() //destructeur { //delete this; }
- Inutile

main.cpp
Code:

1 2 const int SCREEN_WIDTH = 1400; const int SCREEN_HEIGHT = 900;
- unsigned int ?

Code:

char texte[15];

- Utiliser std::string.

PS : Si tu es en C++, "abandonne" la SDL et jette un œil à la SFML.

11/07/2014, 20h58
guitz

Merci infiniment Ehonn pour cette réponse aussi complète :)

Je vais m'y atteler demain, tu me fais progresser à vitesse grand V là :f1::alerte: :ccool:

Je te tiens au courant, bonne soirée.
11/07/2014, 22h42
Médinoc

Ça sent mauvais, ça:

Citation:

void add_ball(std::vector<Ball*> array_balls);

Pourquoi passer un vecteur à une fonction censée ajouter une seule balle?

Si tu compiles en C++11, tu devrais passer un unique_ptr<Ball> en paramètre, pour montrer que la fonction passe la responsabilité de détruire la balle à l'objet ball_manager.

Bonjour,
Citation:
Envoyé par Ehonn
C'est probablement très bien pour du C-- :E

Voici quelques remarques rapides.

Ball_manager.h

Code:

Ball_manager(std::vector<Ball*> array_balls, std::vector<SDL_Rect> spriteBallsClips, int num_balls, int min_velocity, int max_velocity);

- Ne pas utiliser de pointeur nu. Tu as std::unqiue_ptr, std::shared_ptr et std::reference_wrapper pour ça. http://en.cppreference.com/w/
- Il manque des const :
- - Si tu as besoin de modifier un objet, tu le passes par référence : T &.
- - Si tu n'as pas besoin de le modifer, tu le passes par référence sur membre constant : T const &, sauf pour les types de base car une copie n'est pas "coûteuse".
- - Les fois où tu fais une copie sont rares : lorsque tu as besoin de modifier la copie passée.
(Ces remarques sont valables dans le reste du code)

...

Ball_manager.cpp
Code:

1 2 3 4 5 6 7 8 9 Ball_manager::Ball_manager(std::vector<Ball*> array_balls, std::vector<SDL_Rect> spriteBallsClips , int num_balls, int min_velocity, int max_velocity){ m_array_balls = array_balls; m_spriteBallsClips = spriteBallsClips; s_num_balls = num_balls; m_min_velocity = min_velocity; m_max_velocity = max_velocity; }
- Utilise la liste d'initialisation : FAQ C++ - Mes constructeurs doivent-ils utiliser les listes d'initialisation ou l'affectation ?.
Code:

1 2 3 4 5 6 7 Ball_manager::~Ball_manager() { // Version if array_balls is a C-array for(int nb=0; nb < Ball_manager::s_num_balls; nb++){ delete m_array_balls[nb]; m_array_balls[nb] = NULL; } }
- La responsabilité de la mémoire n'est pas très clair (c'est un des avantages de std::unqiue_ptr, std::shared_ptr et std::reference_wrapper).
- Généralement, c'est celui qui alloue qui supprime, alors que ici tu as un transfère non évident de responsabilité.
- Pour avoir la taille d'un tableau (std::vector), c'est ton_vecteur.size().
- En C++11, on utilise nullptr.

Ball.h
- Identation
- Utilise des références [sur membre constant], pas des pointeurs.
Tout ceci est compliqué pour le débutant que je suis (je viens de regarder quelques tutos vite fait), il y a un gros cap à passer, tout ceci est très obscur pour moi. si je m'écoutais je continuerai à coder "crade", mais d'un autre côté il faut que je prenne sur moi (et surtout du temps !) et que je m'y mette un jour :(
Citation:
Envoyé par Ehonn
Code:

~Ball();

- Inutile : Si le destructeur en fait rien, autant ne pas l'écrire, le compilateur le générera pour toi.
Code:

1 2 3 4 Ball::~Ball() //destructeur { //delete this; }
- Inutile
Pourquoi le destructeur est inutile ?
Dans main.cpp:
Code:

1 2 delete ball_manager1; ball_manager1 = NULL;
le delete appelle bien le destructeur de Ball_manager ?
Code:

1 2 3 4 5 6 Ball_manager::~Ball_manager() { for(int nb=0; nb < m_num_balls; nb++){ delete m_array_balls[nb]; m_array_balls[nb] = NULL; } }
et delete m_array_balls[nb]; appelle bien le destructeur de Ball ?
Code:

1 2 3 4 Ball::~Ball() //destructeur { //delete this; }
Si c'est le cas je met quoi dans celui-ci ?

Citation:

Envoyé par Ehonn

PS : Si tu es en C++, "abandonne" la SDL et jette un œil à la SFML.

Pour quelles raisons ?

PS: pour tout ce que je n'ai pas cité, j'ai appliqué tes conseils, merci

Citation:

Envoyé par Médinoc

Ça sent mauvais, ça:

Pourquoi passer un vecteur à une fonction censée ajouter une seule balle?

Si tu compiles en C++11, tu devrais passer un unique_ptr<Ball> en paramètre, pour montrer que la fonction passe la responsabilité de détruire la balle à l'objet ball_manager.

Merci, je vais appliquer ton conseil :ccool:

13/07/2014, 07h09
Sytten

Bonjour,

Lorsque le destructeur ne fait pas de tâches spéciales (comprendre qu'il n'y a pas d'instructions dans l'implémentation de celui-ci) comme c'est le cas dans votre exemple, il n'est pas nécessaire de l'écrire. Le compilateur se chargera lui-même d'en créer un qui sera adapté à votre classe. Bien entendu celui-ci sera appelé lors des delete. L'écrire revient donc dans ce cas à une perte de temps et rend le code moins lisible.

Pour ce qui est de la SDL et de la SFML, il est préférable d'utiliser cette dernière (SFML) en C++ notamment puisqu'elle a été écrite en C++ (contrairement à la SDL qui est écrite en C) et est donc plus à même de répondre aux exigences posées par ce langage (ou plutôt devrais-je dire: par ses paradigmes particulièrement celui orienté objet).
13/07/2014, 07h21
foetus

Citation:

Envoyé par Sytten

Lorsque le destructeur ne fait pas de tâches spéciales (comprendre qu'il n'y a pas d'instructions dans l'implémentation de celui-ci), ) comme c'est le cas dans votre exemple

1) c'est imprécis 2) il utilise un vecteur contenant des pointeurs pointant sur des types Ball :mrgreen:

Il faut dire: "Il faut coder un destructeur lorsque l'allocation des attributs n'est pas automatique"

Et évidemment sans les "smart pointeurs" proposés par Ehonn, le destructeur doit être codé :mrgreen:
13/07/2014, 09h07
Médinoc

Ou plus simplement "Si ton destructeur est public non-virtuel et si son corps est vide, pourquoi l'écrire?"