Just finished reading up to Classes(II)

closed account (NyhkoG1T)
I wanted to post my practice CPP file and make sure I am doing everything properly. Everything does compile and execute as designed, however, what im looking for is critique on best programming practices, code design, do's and dont's.

What I feel is wrong
This is my experiment project and I do feel that overall code organization is very much lacking and is a bit unreadable.

My experience in C++
NONE lol

My experience coding in general
Visual Basic from the pre .NET days (ver. 3-6), and scripting languages like Python, Ruby, and Linux BASH.

Other Notes
The dynamic memory allocation example below is straight from the tutorial. I only changed the output logic to not add a comma at the last index

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
// testingcode.cpp : Defines the entry point for the console application.
//

#include <stdio.h>
#include <iostream>
#include <istream>
#include <Windows.h>
#include <new>

using namespace std;

struct v3 {
	float X;
	float Y;
	float Z;
}Vector3;

class test {
private:
	int x;
	int y;
	int z;
	char *poot;
	
public:
	static int myglobal;
	void SetX(int xx) { x = xx; }
	void SetY(int yy) { y = yy; }
	void SetZ(int zz) { z = zz; }
	void SetCHAR(char *p) { poot = p; }
	int GetX() { return x; }
	int GetY() { return y; }
	int GetZ() { return z; }
	char *GetCHAR() { return poot; }
	test() { x=0;y=0;z=0; }
	test(int xx, int yy, int zz) { x=xx;y=yy;z=zz;}
	operator int();
	operator char*();
	operator v3();
	test operator+ (test);
	test operator- (test);
}icles;

test test::operator+ (test b) {
	test temp;
	
	temp.x = x + b.x;
	temp.y = y + b.y;
	temp.z = z + b.z;

	return (temp);
}

test test::operator- (test b) {
	// a little joke :)
	test temp;

	temp.x = 999;
	temp.y = 888;
	temp.z = 777;

	return (temp);
}
test::operator int() {
	return x;
}
test::operator v3() {
	v3 poot;
	poot.X = x;
	poot.Y = y;
	poot.Z = z;
	return poot;

	// I know im assigning int to a float
	// which would cause trunication if this was reversed
	// i.e. assigning a floating point to an integer variable
	// although compiler is still throwing possible loss of data warnings

	//Question that has arisen:
	// If a signed int is approx 2million negative through 2million positive
	// and unsigned is approx 0 to 4million positive
	// what if you need negative 4 million to 0? 

	//Also, why is there a 'long' type of variable
	// when it's identical to the use of int?
	// they're both 4 bytes, and either signed or unsigned, have the same reach
}
test::operator char*() {
	return poot;
}

//********** PROTOTYPE FUNCTIONS ***********
int main();
void dynamic_memory_example();
int multiple(int x, int y);
float multiple(float x, float y);
double multiple(double x, double y);

int divide(int x, int y);
float divide(float x, float y);
double divide(double x, double y);

int subtract(int x, int y);
float subtract(float x, float y);
double subtract(double x, double y);

int addition(int x, int y);
float addition(float x, float y);
double addition(double x, double y);

void outresult(char * message, int (*multiple)(int,int));
void SetV(float xx, float yy, float zz);
void GetV(float *xx, float *yy, float *zz);

//********* END PROTOTYPE FUNCTIONS ***********

//********* DEFINED FUNCTIONS *************

int multiple(int x, int y) { return x*y; }
float multiple(float x, float y) { return x*y; }
double multiple(double x, double y) { return x*y; }

int divide(int x, int y) { return x/y; }
float divide(float x, float y) { return x/y; }
double divide(double x, double y) { return x/y; }

int subtract(int x, int y) { return x-y; }
float subtract(float x, float y) { return x-y; }
double subtract(double x, double y) { return x-y; }

int addition(int x, int y) { return x+y; }
float addition(float x, float y) { return x+y; }
double addition(double x, double y) { return x+y; }

void doublethenumber(int *num2double);


void doublethenumber(int *num2double) { //argument takes address of an already defined integer
	*num2double *= 2; //dereferences the address received and multiples by 2

	// function doesn't need to return
	// the value of the variable in which we received the address of has changed

	// playing with pointer/reference/dereference operators
}
void outresult(char * message, int x, int y, int (*functocall)(int,int)) {
	int b;
	b = (*functocall)(x,y);
	cout << message << " " << b;
}


void SetV(float xx, float yy, float zz) {
	Vector3.X = xx; Vector3.Y = yy; Vector3.Z = zz;
}

void GetV(float *xx, float *yy, float *zz) {
	*xx = Vector3.X;
	*yy = Vector3.Y;
	*zz = Vector3.Z;
}
void dynamic_memory_example() {
	int i, n;
	int * p;
	
	cout << "How many numbers would you like to type?";
	cin >> i;
	p = new (nothrow) int[i];
	if(p == 0) {
		cout << "Error: Memory could not be allocated." << endl;
	}
	else
	{
		for ( n=0; n<i; n++ ) {
			cout << "Enter a number: ";
			cin >> p[n];
		}
		cout << "You have entered: ";
		for ( n=0; n<i;n++ ) {
			if(n != (i - 1)) {
				cout << p[n] << ", ";
			}else{
				cout << p[n] << endl;
			}
		}
		delete[] p;
	}
}

//****************** END OF DEFINED FUNCTIONS *************************

int main() {


	int br = 10;
	doublethenumber(&br);
	
	cout << br;

	return 0;
}
After a quick scan I noticed a couple of things.

First main() doesn't need a prototype.

Second you see to be including sever include files you don't need. The only header this program seems to need is the <iostream> header.

Also in a C++ program <stdio.h> is depreciated, you should use the C++ versions of these C standard headers (<cstdlib>).

A couple of your functions have parameters that you're not using and some of your member functions have parameter names or are using variables that are named the same as your class variable names, this will probably cause problems. Using parameters or variables with the same names as your member variables hide the member variables and can cause subtle bugs that are hard to find.


closed account (NyhkoG1T)
I definitely do need to clean out my includes lol. I do believe the #include <new> is for the dynamic memory function?

I just saw that my prototype for "outresult" is erroneous and doesn't reflect the actual function I wrote. I think I just forgot to update the prototype. Not sure why my compiler didn't throw something up?

I glanced at the differences between stdio and cstdlib, and cstdlib seems to be more for string conversions, pseudo-random sequences and memory allocation while stdio is for file input/output and formatting output (which I use sprintf_s quite a bit)

I do need to change up my variable usage and get away from X,Y,Z for everything ... it is quite confusing.

The only other function that I could see where I didn't use the parameters is the member function of my class where I overload the subtraction operator. I was going to write this properly at first, then I decided to change the output to something stupid. lol

are you using all of those functions you've declared?
I do believe the #include <new> is for the dynamic memory function?

Not unless you're defining your own new handler, otherwise you don't need to include <new>.

I just saw that my prototype for "outresult" is erroneous and doesn't reflect the actual function I wrote. I think I just forgot to update the prototype. Not sure why my compiler didn't throw something up?

Maybe you need to increase your compiler warning level. Here is what my compiler says:
main.cpp||In function ‘void outresult(char*, int, int, int (*)(int, int))’:|
main.cpp|146|warning: no previous declaration for ‘void outresult(char*, int, int, int (*)(int, int))’ [-Wmissing-declarations]|


while stdio is for file input/output and formatting output (which I use sprintf_s quite a bit)

First you should really start using std::string instead of the C-strings. But even if you do use sprint() you should be using the C++ header <cstdio> not the C header <stdio.h>.

But beware of the function that names a local variable with the same name as one of your member variables.
1
2
test::operator v3() {
	v3 poot;

The variable poot is a class member variable. Also that function signature doesn't look correct.

While the function overloading is okay, you may want to consider templates instead.

Last edited on
I am kind of assuming that you're deliberately using pointers for practice, for out parameters, where C++ code would generally use references, and other such things?

But the biggest problem with your code is that it is rather random, and that almost all the code is uncalled for as it stands. After the optimizer has finished with the code, you might as well have just written:

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
#include <iostream>
using namespace std;

void doublethenumber(int *num2double) {
	*num2double *= 2;
}

int main() {
	int br = 10;
	doublethenumber(&br);
	
	cout << br;

	return 0;
}


(a modern linker will throw away all the irrelvant machine code as part of the link phase; if you're using Visual C++, you'll see the default configuration for the release build passes the /Gy flag - Enable Function-Level Linking - to the compiler, which adds the info to the generated .obj files that the linker needs to spot and throw away unused functions.)

There are plenty of comments that could be made about your code, but some of them are about how things are done in a more general way, rather than the syntactic correctness. For example, the cast operators are ok syntactically, but not from a meaning point of view. But to comment further on this issue I would need a more meaningful example.

It would be better if you carved up your code into a set of programs, one for each of your (groups of) experiments.

To see where you've got to, you should try to come up with a program that solves a real, small problem.

But note there is absolutely no point "forward declaring" a function if you then go on to define it before first use. e.g. use either

1
2
3
4
5
6
7
8
9
10
11
12
#include <iostream>
using namespace std;

void doublethenumber(int *num2double) {
	*num2double *= 2;
}

int main() {
	doublethenumber(&br);
	cout << br;
	return 0;
}


or

1
2
3
4
5
6
7
8
9
10
11
12
13
14
#include <iostream>
using namespace std;

void doublethenumber(int *num2double);

int main() {
	doublethenumber(&br);
	cout << br;
	return 0;
}

void doublethenumber(int *num2double) {
	*num2double *= 2;
}


(for such a small function I would use the first of thes two versions, and mark it explicitly as inline.)

Andy
Last edited on
i just have one little note:
i can see that you overloaded some functions to do the same algorithm on different data types.
you should consider using template functions, they're there to ease such thing.
closed account (NyhkoG1T)
I know!!! I just read through friendship and inheritance, polymorphism, abstract base classes, templates and namespaces today and was excited to learn about them. :)

I've yet to play with templates extensively so at this point, writing a template for that is beyond the scope of my knowledge. :(

After a few more read throughs of this section and some practice sessions im sure i'll get it though... someone mentioned #include <vector> the other day and I became quite side tracked lol
Topic archived. No new replies allowed.