rvalue vs. lvalue

closed account (zwA4jE8b)
I have a function that is called many times in my game. It is in the main loop.
I am wondering if having a previously initialized int variable is faster than calculating an rvalue. The thing is the 2nd code snippet also uses a preset int also.

All in all my game is simple and either way will i do not think this will affect the speed because of how minor the calculation is, but I would like to know just for general knowledge which is faster.


1
2
3
4
5
6
7
8
9
void animate_snake(const HDC& hDC)
	{
		FillRect(hDC, &_segments[0], _bluebrush);

		if (_counter == _tocount) //HERE
			FillRect(hDC, &_old, _whitebrush);
		else
			_counter++;
	}


or

1
2
3
4
5
6
7
8
9
void animate_snake(const HDC& hDC)
	{
		FillRect(hDC, &_segments[0], _bluebrush);

		if (_counter == (_lvl * 8) - 1) //HERE
			FillRect(hDC, &_old, _whitebrush);
		else
			_counter++;
	}
The first snippet does two reads and one comparison. The second one does two reads, a comparison, a multiplication and a subtraction, so, of course, it will take negligibly shorter time.

A more interesting question would be to compare
1
2
3
float dx = ax-bx;
float dy = ay-by;
float distance = sqrt( dx*dx+dy*dy );

with
float distance = sqrt( (ax-bx)*(ax-bx)+(ay-by)*(ay-by) );

I think I benchmarked this and found that the second one is faster. My guess is that ax-bx is not calculated twice but kept in some register. I may be remembering this wrong though..
You shouldn't waste your time worrying about which will be faster.

Any speed difference between the two is theoretical at best. Until you actually see a performance difference, there is no reason to concern yourself with it.

Making a separate _counter variable will likely make maintenance more difficult and your code more confusing.

So the real question here is... is it worth it to make your code more confusing in order to get a theoretical performance increase that nobody could ever notice?

I'd say no.




Clear code is more important. Write code that is easy to understand, follow, and bugfix. Worry about optimizations when you have to.
I had a quick look at hamsterman's sqrt example. With Visual C++ 2008, /O2 /Oi /Oy optimization, the disassembly is identical.

I prefer more local variable as a rule, as it's easier to spot when things go wrong. The compiler invariably has it's own ideal about how many variables it wants. Which can include adding additional temporarires (with obscure names), so avoiding variable doesn't always work!

Andy

P.S. For people who don't know VC++ switches, /O2 = optimise for speed, /Oi = enable intrinsics, /Oy = enable frame pointer optimization
closed account (zwA4jE8b)
Thanks guys, and disch that makes sense.
Although my snakengine is simple, I have just been trying to optimize it.

If anyone has any constructive criticism, it would be much appreciated.


snakemain.cpp
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
//////////////////////////////////////////////////////////////////////////////////////////////////////////
//Michael Ervin - Windows based graphical snake game - 7/29/2011
//////////////////////////////////////////////////////////////////////////////////////////////////////////

#include <Windows.h>
#include "resource.h"
#include "snakengine.h"

//////////////////
game _game;
//////////////////

LRESULT CALLBACK MessageProc(HWND hWnd, UINT message, WPARAM wParam, LPARAM lParam);

int WINAPI WinMain(HINSTANCE hInstance,
                   HINSTANCE hPrevInstance,
                   LPSTR lpCmdLine,
                   int nCmdShow)
{
	WNDCLASSEXA wc;

	ZeroMemory(&wc, sizeof(WNDCLASSEX));

	wc.cbSize = sizeof(WNDCLASSEX);
	wc.style = CS_HREDRAW | CS_VREDRAW;
	wc.lpfnWndProc = MessageProc;
	wc.hInstance = hInstance;
	wc.hCursor = LoadCursor(NULL, IDC_ARROW);
	wc.hIcon = LoadIcon(GetModuleHandle(NULL), MAKEINTRESOURCE(IDI_ICON1)); 
	wc.lpszClassName = "SNAKE";

	RegisterClassExA(&wc);

	HWND hWnd = CreateWindowExA(NULL,
                          "SNAKE", "Snake",
                          WS_SYSMENU | WS_BORDER,
                          0, 45,
                          screen_width, screen_height,
                          NULL, NULL,
                          hInstance,
                          NULL);

	ShowWindow(hWnd, nCmdShow);
	HDC hDC = GetDC(hWnd);

	MSG msg;

	_game.game_getvars(hDC, hWnd);
	_game.game_setstate('i');

	while(TRUE)
	{
		if(PeekMessage(&msg, NULL, 0, 0, PM_REMOVE))
		{
			if(msg.message == WM_QUIT)
				break;
			TranslateMessage(&msg);
			DispatchMessage(&msg);
		}
		else
		{
			_game.game_state();
			Sleep(75);
		}
	}
	return msg.wParam;
}

LRESULT CALLBACK MessageProc(HWND hWnd, UINT message, WPARAM wParam, LPARAM lParam)
{
	switch(message)
	{
		case WM_DESTROY:
		{
			PostQuitMessage(0);
			return 0;
		} break;
		case WM_KEYDOWN:
		{
			switch (wParam)
			{
			case _up:
			case _down:
			case _left:
			case _right:
				_game.game_input(wParam);
				break;
			case _pause:
				_game.game_setstate('t');
				break;
			case _esc:
				_game.game_setstate('o');
				break;
			}
			return 0;
		} break;
     }
     return DefWindowProcA(hWnd, message, wParam, lParam);
 }
closed account (zwA4jE8b)
snakengine.h

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
142
143
144
145
146
147
148
149
150
151
152
153
154
155
156
157
158
159
160
161
162
163
164
165
166
167
168
169
170
171
172
173
174
175
176
177
178
179
180
181
182
183
184
185
186
187
188
189
190
191
192
193
194
195
196
197
198
199
200
201
202
203
204
205
206
207
208
209
210
211
212
213
214
215
216
217
218
219
220
221
222
223
224
225
226
227
228
229
230
231
232
233
234
235
236
237
238
239
240
241
242
243
244
245
246
247
248
249
//Snake object header
#ifndef H_snakengine
#define H_snakengine

#define screen_width 800
#define screen_height 600

#include <vector>
#include <ctime>

#include <sstream>
#include <string>

COLORREF _blue = RGB(0,0,255);
COLORREF _red = RGB(255,0,0);
COLORREF _white = RGB(255,255,255);
COLORREF _black = RGB(0,0,0);
COLORREF _green = RGB(0,255,0);
COLORREF _purple = RGB(200,50,200);
const int _rectwidth = 10, _rectheight = 10, _bordertop = 5, _borderleft = 5,
		  _borderbottom = screen_height - 65, _borderright = screen_width - 9;
const DWORD _up = 0x57, _down = 0x53, _left = 0x41, _right = 0x44, _pause = 0x50,
			_yes = 0x59, _no = 0x4E, _esc = 0x1B;
HBRUSH _bluebrush = CreateSolidBrush(_blue);
HBRUSH _whitebrush = CreateSolidBrush(_white);
HBRUSH _blackbrush = CreateSolidBrush(_black);
HBRUSH _greenbrush = CreateSolidBrush(_green);

std::string convstr(const int& t){std::stringstream itoa; itoa << t; return itoa.str();}
std::vector<RECT> _foodgrid, _obstaclegrid;

class level
{
public:
	int _numfood, _numobstacle;

	level(HDC hDC, int _lvl, int _scr):_hDC(hDC), _level(_lvl), _score(_scr){}

	void init_level()
	{
		draw_level();
		fill_foodobstaclegrid();
	}
	void update_info(int& _score)
	{
		std::string _slevel = convstr(_level);
		std::string _sscore = convstr(_score);
		TextOut(_hDC, 60, screen_height - 55, _slevel.c_str(), _slevel.length());
		TextOut(_hDC, 145, screen_height - 55, convstr(_score).c_str(), _sscore.length());
	}
	void end_game()
	{
		int _width;
		SetTextColor(_hDC, _purple);
		std::string _gameover = "Game Over";
		std::string _playagain = "Do you want to play again? (y / n)";
		_width = (screen_width / 2) - 40;
		TextOut(_hDC, _width, screen_height / 2, _gameover.c_str(), _gameover.length());
		_width = (screen_width / 2) - 100;
		TextOut(_hDC, _width, (screen_height / 2) + 50, _playagain.c_str(), _playagain.length());
		SetTextColor(_hDC, _black);
	}
private:
	int _level, _score;
	const HDC _hDC;
	void draw_level()
	{
		RECT _back = {0,0,screen_width - 6,screen_height - 60};
		RECT _front = {10,10,screen_width - 10,screen_height - 70};
		FillRect(_hDC, &_back, _blackbrush);
		FillRect(_hDC, &_front, _whitebrush);
	}
	void fill_foodobstaclegrid()
	{
		_numfood = _level * 4;
		_numobstacle = _level * 3;
		int _top, _left, i, j;
		srand(time(0));

		std::vector<RECT> _used;
		RECT _usedone = {390, 290, 400, 300};
		_used.push_back(_usedone);

		for(i = 0; i < _numfood; i++)
		{
			_top = 10 + (rand() % (_borderbottom - 20));
			if(_top % 10 != 0)
				_top -= _top % 10;
			_left = 10 + (rand() % (_borderright - 20));
			if(_left % 10 != 0)
				_left -= _left % 10;
			RECT _food = {_left, _top, _left + 10, _top + 10};
			for(j = 0; j < _used.size(); j++)
				if(_food.top == _used[j].top && _food.left == _used[j].left)
				{
					i--;
					break;
				}
				else
				{
					_foodgrid.push_back(_food);
					_used.push_back(_food);
					FillRect(_hDC, &_food, _greenbrush);
					break;
				}
		}

		for(i = 0; i < _numobstacle; i++)
		{
			_top = 10 + (rand() % (_borderbottom - 20));
			if(_top % 10 != 0)
				_top -= _top % 10;
			_left = 10 + (rand() % (_borderright - 21));
			if(_left % 10 != 0)
				_left -= _left % 10;
			RECT _obstacle = {_left, _top, _left + 10, _top + 10};
			for(j = 0; j < _used.size(); j++)
				if(_obstacle.top == _used[j].top && _obstacle.left == _used[j].left)
				{
					i--;
					break;
				}
				else
				{
					_obstaclegrid.push_back(_obstacle);
					_used.push_back(_obstacle);
					FillRect(_hDC, &_obstacle, _blackbrush);
					break;
				}
		}
		TextOut(_hDC, 15, screen_height - 55, "Level: ", 7);
		TextOut(_hDC, 100, screen_height - 55, "Score: ", 7);
	}
};

class snake
{
	int _lvl, _length, _counter, _tocount, i;
	RECT _old;
	std::vector<RECT> _segments;
	char _ret;
	
public:
	char _direction;
	snake(int _level):_lvl(_level), _length(_lvl*8), _direction(NULL), _counter(0), _ret('n'), _tocount((_lvl * 8) - 1){}

	void init_snake()
	{
		_segments.resize(0);
		for(i = 0; i < _length; i++)
		{
			RECT _newseg = {390, 290, 390 + _rectwidth, 290 + _rectheight};
			_segments.push_back(_newseg);
		}
	};
	void show_head(const HDC& hDC)
	{
			FillRect(hDC, &_segments[0], _bluebrush); 
	}
	void next_pos()
	{
		_old = _segments.back();

		for(i = _length - 1; i > 0; i--)
			_segments[i] = _segments[i - 1];
		
		switch (_direction)
		{
		case _right:
			_segments[0].left += _rectwidth;
			_segments[0].right += _rectwidth;
			break;
		case _left:
			_segments[0].left -= _rectwidth;
			_segments[0].right -= _rectwidth;
			break;
		case _up:
			_segments[0].top -= _rectheight;
			_segments[0].bottom -= _rectheight;
			break;
		case _down:
			_segments[0].top += _rectheight;
			_segments[0].bottom += _rectheight;
			break;
		}
	}
	void animate_snake(const HDC& hDC)
	{
		FillRect(hDC, &_segments[0], _bluebrush);

		if (_counter == _tocount)
			FillRect(hDC, &_old, _whitebrush);
		else
			_counter++;
	}
	void add_seg()
	{
		RECT _newseg = _segments[_length - 1];
		_segments.push_back(_newseg);
		_segments.push_back(_newseg);
		_length+= 2;
	}
	char collisions()
	{
		_ret = 'n';

		switch (_direction)
		{
		case _up:
			if(_segments[0].top <= _bordertop)
			_ret = 'o';
			break;
		case _down:
			if(_segments[0].bottom >= _borderbottom)
			_ret = 'o';
			break;
		case _left:
			if(_segments[0].left <= _borderleft)
			_ret = 'o';
			break;
		case _right:
			if(_segments[0].right >= _borderright)
			_ret = 'o';
			break;
		}

		for(i = 0; i < _foodgrid.size(); i ++)
			if(_segments[0].top == _foodgrid[i].top && _segments[0].left ==_foodgrid[i].left)
			{
				_ret = 'f';
				break;
			}

		for(i = 0; i < _obstaclegrid.size(); i ++)
			if(_segments[0].top == _obstaclegrid[i].top && _segments[0].left ==_obstaclegrid[i].left)
			{
				_ret = 'o';
				break;
			}

		for(i = 1; i < _length; i++)
			if(_segments[0].top == _segments[i].top && _segments[0].left ==_segments[i].left)
			{
				_ret = 'o';
				break;
			}
		return _ret;
	}
};
Last edited on
closed account (zwA4jE8b)
snakengine.h continued

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
142
143
144
145
146
class game
{
	level* _LVL;
	snake* _SNK;
	HDC _hDC;
	int _level, _score, _foodeaten, _foodneeded;
	char _collision, _state;
	HWND _hWnd;

public:
	game():_level(1), _score(0){}

	void game_setstate(char state)
	{
		_state = state;
	}
	void game_state()
	{
		switch (_state)
		{
		case 'i':
			game_init();
			break;
		case 'r':
			game_reinit();
			break;
		case 'p':
			game_play();
			break;
		case 't':
		case 'u':
			game_pause();
				break;
		case 'o':
			game_over();
			break;
		}
	}
	void game_getvars(HDC hDC, HWND hWnd)
	{
		_hWnd = hWnd;
		_hDC = hDC;
	}
	void game_init()
	{		
		_foodeaten = 0;

		_LVL = new level(_hDC, _level, _score);
		_LVL->init_level();
		_LVL->update_info(_score);

		_SNK = new snake(_level);
		_SNK->init_snake();
		_SNK->show_head(_hDC);

		_foodneeded = _LVL->_numfood;

		_state = 'p';
	}
	void game_reinit()
	{
		_foodgrid.resize(0);
		_obstaclegrid.resize(0);
		delete _SNK;
		delete _LVL;
		_state = 'i';
	}
	void game_input(WPARAM _direction)
	{
		switch (_direction)
		{
		case _up:
			_SNK->_direction = _up;
			break;
		case _down:
			_SNK->_direction = _down;
			break;
		case _left:
			_SNK->_direction = _left;
			break;
		case _right:
			_SNK->_direction = _right;
			break;
		}
	}
	void game_play()
	{
		if(_SNK->_direction != NULL)
		{
			_SNK->next_pos();
			_SNK->animate_snake(_hDC);
			switch (_SNK->collisions())
			{
			case 'f':
				_foodeaten++;
				_score += _level * 5;
				_SNK->add_seg();
				_LVL->update_info(_score);
				break;
			case 'o':
				_state = 'o';
				break;
			case 'n':
				break;
			}
			if(_foodeaten == _foodneeded)
			{
				_level++;
				_state = 'r';
			}
		}
	}
	void game_pause()
	{
		static bool _paused = false;
		if(_state == 't')
			_paused = !_paused;
		if(!_paused)
			_state = 'p';
		else
			_state = 'u';
		Sleep(10);
	}
	void game_over()
	{
		_state = 'a';
		_LVL->end_game();
		while(true)
		{
			if(GetAsyncKeyState(_yes))
			{
				_level = 1;
				_score = 0;
				_state = 'r';
				break;
			}
			else if(GetAsyncKeyState(_no))
			{
				PostQuitMessage(0);
				break;
			}
			Sleep(10);
		}
	}
};
#endif 
closed account (zwA4jE8b)
how you like me now disch... no more console! :)

p.s. thank you for the indirect challenge. I have learned much, and that is not sarcastic.
Last edited on
Your header -- snakengine.h -- is out of control.

Header files are supposed to be just for definitions, not implementations. And your classes are getting big enough that they merit their own individual files.

As a general rule, you should have one class per h/cpp file, for all non-trivial classes. In you case, you could do with snake.h, snake.cpp, level.h, level.cpp, game.h, game.cpp

Another rule is that a class definition should be about an IDE page long at most. If it's getting out of hand, somethings going wrong with you object modelling!

Splitting the source will make you requilds fast. Esp. when more classes turn up.

Andy

P.S. templated classes do require their implementation in the header. But then templated classes should be on the small side.
closed account (zwA4jE8b)
I am picking up what you are putting down.
Except when I split them, for example, I leave all the class and and function prototypes in snakengine.h
and put all the function definitions in snakengine.cpp I get a lot of undefined errors. Can you give me and example of how to split them?
closed account (zwA4jE8b)
do I leave the initializer list in the header or in the cpp
Re: the inializer list

It depends on the size of constructor. If the consructor is small I leave it inline in the class body.

If it gets too large (a subjective call...) I just declare the it in the header and define it in the cpp file

The initializer list is part of the definition, so it moves into the cpp file.

e.g.

1
2
3
4
5
6
7
8
9
10
11
// example.h

class Example
{
private:
    int index;
public:
    Example() : index(0)
    {
    }
};


Or

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
// example.h

class Example
{
private:
    int index;
public:
    Example();
};

// example.cpp

... standard header go here ...

#include "example.h"

Example::Example() : index(0)
{
}

Last edited on
Topic archived. No new replies allowed.