C++

Forum

 
General Class Structure

jimctr (3)
I am trying to update my old C code to C++ , and trying to grasp C++ at the same time.

It's not clear to me when creating a class, which variables I should declare as private (which are basically behaving as global variables to member functions) verses those I should simply pass as parameters.

1) Do I take ALL parameters and return values of functions I am converting from C to C++ and make them private members?

2) If I don't have to make private members of all parameters and returned values of functions, what criteria do I use to determine which parameters should be privatized to the class versus those which can simply be supplied externally? e.g, cTest.SetVals(4,5), where the variables set to 4 and 5 respectively are not necessarily member variables in the sense that they are only defined in the context of the function, not as stand alone variables in the class.

3) Is my approach wrong to treat the member variables as global variables to the member function as in the example below: mVal, mDrop and mDun were defined as private variable members but they are not passed as parameters but simply show up as global references.

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
 
//drop => drop value.   dun: //0 = MOA, 1 = MILs
void AS1113::SetMOA(){
	
	float val;
	unsigned char step;

	
	#define MAX_POS		3     //supports a maximum MOA 2*3 = 6
	
		
	if(mDun)		//If in mils
	val = mDrop * MIL_CONVT;   //convert to MOA
	else {val = mDrop;}
	
	step = (unsigned char)floor((MAX_POS - (val/MOA_RESOLUTION)) + 0.5);
		
	mVal = TRAY >> step;
	
	WriteLED();
		
}



4) If you are not supposed to use these member variables globally, is the only way to pass these values by self-referencing the class, as in the example below? By self-referencing, I mean that the class basically has to pass an object created from itself as a parameter.


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
 
 

void AS1113::WriteLED(AS1113& a){
	
	int temp;

	//Clock in LSB First
	
	gpio_set_gpio_pin (OEN);
	
	for(short x=0; x < LED_CYCLES; x++){
		temp = a.mVal & LSB_MASK;
		if (temp)
			gpio_set_gpio_pin (SDI);
		else 
			{gpio_clr_gpio_pin (SDI);}
		
		cpu_delay_us(1, FOSC0);
		gpio_set_gpio_pin (SCK);
		cpu_delay_us(1, FOSC0);
		gpio_clr_gpio_pin (SCK);
		a.mVal = a.mVal >> 1;
	}//end for
	
	
	gpio_clr_gpio_pin (SDI);
	//Pulse LOAD HIGH for 1us
	cpu_delay_us(1, FOSC0);
	gpio_set_gpio_pin (LOAD);
	cpu_delay_us(1, FOSC0);
	gpio_clr_gpio_pin (LOAD);
	
	//Pulse OE LOW for 1us
	cpu_delay_us(1, FOSC0);
	gpio_clr_gpio_pin (OEN);
	
	
}



Last edited on
Stewbond (2827)
Make the members private so that you don't need to pass them as parameters between methods. Add getters or setters only if you need to modify those contents directly outside of your class. If you have a specific way of validation, or setting multiple values with one method, implement that instead of a setter.
jimctr (3)
If I do make self-referencing as in the example below, it becomes very confusing to me;

.hpp file
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
 
#define TRAY					0x80000000		//LED bit
#define LSB_MASK				0x00000001
#define LED_CYCLES						32
#define MOA_RESOLUTION					2.0
#define MIL_CONVT						3.438   //Multiply MILs by this to get MOA

//Class Definition
class AS1113 {
	
	
	private:
		int mVal;
		float mDrop;
		bool mDun;	
		
	public:
		//default constructor
		AS1113();
		
		//Setters
		void SetVal(int val) {mVal = val;}
		void SetDrop(float drop) {mDrop = drop;}
		void SetDun(bool dun) {mDun = dun;}	
		
		//Getters
		int GetVal(){return mVal;}
		float GetDrop(){return mDrop;}
		bool GetDun(){return mDun;}
	
		
		void WriteLED(AS1113&);
		void DisableLED();
		void SetMOA(AS1113&);
		
	
	};


.cpp File

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
 
#include "AS1113.hpp"

AS1113::AS1113(){
	DisableLED();	
}


void AS1113::DisableLED(){
	gpio_set_gpio_pin (OEN);
}



void AS1113::WriteLED(AS1113& a){
	
	int temp;

	//Clock in LSB First
	
	gpio_set_gpio_pin (OEN);
	
	for(short x=0; x < LED_CYCLES; x++){
		temp = a.mVal & LSB_MASK;
		if (temp)
			gpio_set_gpio_pin (SDI);
		else 
			{gpio_clr_gpio_pin (SDI);}
		
		cpu_delay_us(1, FOSC0);
		gpio_set_gpio_pin (SCK);
		cpu_delay_us(1, FOSC0);
		gpio_clr_gpio_pin (SCK);
		a.mVal = a.mVal >> 1;
	}//end for
	
	
	gpio_clr_gpio_pin (SDI);
	//Pulse LOAD HIGH for 1us
	cpu_delay_us(1, FOSC0);
	gpio_set_gpio_pin (LOAD);
	cpu_delay_us(1, FOSC0);
	gpio_clr_gpio_pin (LOAD);
	
	//Pulse OE LOW for 1us
	cpu_delay_us(1, FOSC0);
	gpio_clr_gpio_pin (OEN);
	
	
}


//drop => drop value.   dun: //0 = MOA, 1 = MILs
void AS1113::SetMOA(AS1113& b){
	
	float val;
	unsigned char step;

	
	#define MAX_POS		3     //supports a maximum MOA 2*3 = 6
	
	//if(dun)
	//	val = fabs(drop) * MIL_CONVT;   //convert to MOA
	//else {val = fabs(drop);}
	
	if(b.mDun)		//If in mils
	val = b.mDrop * MIL_CONVT;   //convert to MOA
	else {val = b.mDrop;}
	
	step = (unsigned char)floor((MAX_POS - (val/MOA_RESOLUTION)) + 0.5);
	//step = (unsigned char)floor((val/MOA_RESOLUTION) + 0.5);
	
	b.mVal = TRAY >> step;
	
	WriteLED(b);     //DOES B GO HERE?
		
}



What do I do with main? This of course will not compile as I am not passing yet a new object in the main. The main below was written for the global approach. When I treated member variables as global variables everything compiled fine and the code worked. I just don't know if that is the appropriate thing to do.


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
 
#include "AS1113.hpp"
#include "Configure.hpp"

//#define TRAY	0x8000
#define TRAY		0x80000000		//LED bit
#define BTRAY		0x40000000
#define CTRAY		0x00000001



int main(void){
	
	char x;
	unsigned int temp;
	Configure cConfigure;
	
	cConfigure.ConfigurePM ();
	cConfigure.ConfigureGPIO ();

	AS1113 cAS1113;	//Define your instance
	
	for(;;){
		
		temp = TRAY;
		for(x= 0; x < 16; x++){
			cAS1113.SetVal(temp);
			cAS1113.WriteLED();
			cpu_delay_ms(500, FOSC0);
			temp = temp >> 2;
		}
		
		cAS1113.SetVal(BTRAY);
		cAS1113.WriteLED();
		cpu_delay_ms(500, FOSC0);
		
		temp = CTRAY;
		for(x=0; x < 16; x++){
			cAS1113.SetVal(temp);
			cAS1113.WriteLED();
			cpu_delay_ms(500, FOSC0);
			temp = temp << 2;
		}
		
		
		
	}//end for(;;)
	

	
}//end main()





Topic archived. No new replies allowed.