private string setters and getters

[ebonygeek45]

In an effort to understand setter and getter better I did a tutorial on youtube.

I don't think I missed anything but my code is not working and I thought I did what she did. I did a bit of research because I couldn't get the char she had for names to work. So I scratched that and tried it with a string. Can anyone tell me what is wrong with this coded.

CDog.h

#include <iostream>
#include <string>

using namespace std;

#ifndef CDOG_H
#define CDOG_H

class CDog
{
    private:
        string name();    
        int weight();  
        bool rabid();    
    public:
        CDog();
        ~CDog();
    // Attributes
        void setRabid(bool x);
        bool getRabid();
        void setName(string x);
        string getName();
        void setWeight(int x);
        int getWeight();

    // Behaviours 
        void growl();
        void eat();
};

#endif // CDOG_H 

CDog.cpp

#include <iostream>
#include <string>
#include "CDog.h"

using namespace std;

CDog::CDog(int x, string x)
{
    rabid = false;
    weight = x;
    name = x;
}

CDog::~CDog()
{

}

void CDog::setRabid(bool myboolean)
{
    name = myboolean;
}

bool CDog::getRabid();
{
    return(rabid);
}

void CDog::setWeight(int y)
{
    weight = y;
}

int CDog::getWeight();
{
    return(weight);
}

void CDog::setName(string y)
{
    name = y;
}

string CDog::getName();
{
    return(name);
}
// Behaviours

void CDog::eat()
{
    cout << name << "is now eating" <<
            weight++ << endl;
}

void CDog::growl()
{
    cout << "Grrr" << endl;
}

#include <iostream>
#include <conio.h>  
#include <string>
#include "CDog.h"

using namespace std;

int main()
{
    CDog* c1;  
    c1 = new CDog(14, "Bob");   
    CDog c2(7, "Ethel"); 
   
    c2.eat();
    c1->growl();

    getch();
    return 0;
}

[mutexe]

1. 2 variables of the same name:

CDog::CDog(int x, string x)

2. plus you have not defined this constructor in your header file.

3. Main reason appears to be that you don't have a variable declared called 'rabid', its a method, so

rabid = false;

won't compile. Same with 'name', 'weight' etc

it almost looks like you're assuming you have .net-like Property fields?

[mutexe]

Follow this kind of pattern:

// in your header
class Wibble
{
private:
	int m_nAge;
public:
	Wibble();
	~Wibble();
	int getAge();
	void setAge(int age);
};


// in your .cpp
Wibble::Wibble(){}
Wibble::~Wibble(){}

int Wibble::getAge()
{
	return m_nAge;
}

void Wibble::setAge(int age)
{
	m_nAge = age;
}

// in your main.cpp
int main()
{
	Wibble myWibble;

	myWibble.setAge(42);

	return 0;

}

[Peter87]

string name();    
int weight();  
bool rabid(); 

These are three function declarations. If you want them to be variables you should remove the parentheses after the names.

Remove the semicolon from the end of line 25, 35 and 45 in CDog.cpp.

[ebonygeek45]

Never did .net before...Just got hooked on trying C++.

My research said that you only declare in the header and define in the class .cpp.

Are you saying that the constructor definition go in the header?

[mutexe]

the constructor's not the point.

you need a private variable, and a public getter and a public setter.
You didnt have a variable at all.

p.s. my number 2 should have read:

plus you have not declared this constructor in your header file.

[Peter87]

mutexe wrote:

2. plus you have not defined this constructor in your header file.

ebonygeek45 wrote:

Are you saying that the constructor definition go in the header?

He probably meant "declared". You have not declared a constructor that takes an int as the first argument and a string as the second argument, in the class definition.

[mutexe]

indeed Peter. Ta :)

[AbstractionAnon]

My research said that you only declare in the header and define in the class .cpp.

That's correct.

Are you saying that the constructor definition go in the header?

No, that's not what Mutexe was saying in his point #2.

Let me rephrase:
Your constructors don't agree between your .h and .cpp files. They must agree.
.h file liine 16, you declare a default constructor that takes no arguments. There is no matching implementation for this constructor.
.cpp file ine 8, you implement a constructor that takes two arguments, and int and a string. There is no matching declaration for this constructor.

In addition to the errors other have pointed out, you have an issue in your .cpp at line 22. You're trying to assign a bool to a string.

[ebonygeek45]

@Peter87

Well, that was embarrassing, and I know better then those 6 errors.

@mutexe

Thank you for the sample. I understand the process of objects. What I am
trying to figure out is how to get the name to change through the setters and getters.

[Lynx876]

Here's a sample:

main.cpp

// include the class.h
#include "sampleClass.h"

#include <iostream>

int main()
{
	sampleClass myClass;

	// They already have default values,
	// because of the class constructor.
	std::cout << myClass.getText() << '\n';
	std::cout << myClass.getInt() << '\n';

	std::string myString = "Text added via a setter.";
	int myInt = 20;

	// Use the setters to add data to the private variables.
	myClass.setText( myString );
	myClass.setInt( myInt );

	// cout the new values.
	std::cout << myClass.getText() << '\n';
	std::cout << myClass.getInt() << '\n';

	return 0;
}

sampleClass.h

#pragma once

#include <string>

class sampleClass
{
	public:
		sampleClass( std::string t = "Uninitialized", int i = 0 );
		~sampleClass(void);

		// getters/setters
		void setText( std::string &t );
		std::string getText();

		void setInt( int &i );
		int getInt();

		// Create some private variables.
	private:
		std::string text;
		int integer;
};

sampleClass.cpp

#include "sampleClass.h"

sampleClass::sampleClass( std::string t, int i ) : text( t ), integer( i )
{
}

sampleClass::~sampleClass(void)
{
}

void sampleClass::setText( std::string &t )
{
	text = t;
}

std::string sampleClass::getText()
{
	return text;
}

void sampleClass::setInt( int &i )
{
	integer = i;
}

int sampleClass::getInt()
{
	return integer;
}

[ebonygeek45]

I got to take a good look at your sample @Lynx876

@mutexe I got it. I didn't declare my second constructor. I know there are some more errors but that was a big one. Got to take a good look at my main though Thanks.

#include <iostream>
#include <string>//

using namespace std;

#ifndef CDOG_H
#define CDOG_H

    private:
        string name;     
        int weight;   
        bool rabid;   
    public:
        CDog();
        ~CDog();
        CDog(int, string);
    // Attributes
        void setRabid(bool one);
        bool getRabid();
        void setName(string a);
        string getName();
        void setWeight(int x);
        int getWeight();

    // Behaviours 
      
        void growl();
        void eat();

CDog.cpp

#include <iostream>
#include <string>
#include "CDog.h"

using namespace std;

CDog::CDog()
{

}

CDog::~CDog()
{

}

CDog::CDog(int x, string a)
{
    rabid = false;
    weight = x;
    name = a; 
}

void CDog::setRabid(bool myboolean)
{
    name = myboolean;
}

bool CDog::getRabid()
{
    return(rabid);
}

void CDog::setWeight(int y)
{
    weight = y;
}

int CDog::getWeight()
{
    return(weight);
}

void CDog::setName(string b)
{
    name = b;
}

string CDog::getName()
{
    return(name);
}
// Behaviours

void CDog::eat()
{
    cout << name << "is now eating" <<
            weight++ << endl;
}

void CDog::growl()
{
    cout << "Grrr" << endl;
}

#include <iostream>
#include <conio.h>
#include <string>
#include "CDog.h"

using namespace std;

int main()
{
    CDog* c1;
    c1 = new CDog(14, "Bob"); 
    CDog c2(7, "Ethel"); 

    c2.eat();
    c1->growl();

    getch();
    return 0;
}

*/

[LB]

There's actually several false practices in this thread.

1. Getters and setters are bad. For one, you don't need getters - other code should not change its behavior just because the object it is dealing with has a different state; that goes against encapsulation. For two, you should not use setters - it means external code has to calculate the new state of the object, which is also against encapsulation.

Instead, prefer objects that explain themselves (e.g. they have their own function for drawing themself to the screen) and which allow you to change their state indirectly. You can attack an object, but you shouldn't perform calculations for it and set its new health.

Basically, your getters and setters are bad, but your "behaviors" are good.

2. Prefer to define functions inline when possible. If you define everything in the CPP file, that makes it much much harder for the compiler to optimize your code. If everything is available inline in the header with the class definition, then the compiler can easily optimize your code and make it run faster.

Obviously for writing things like libraries, you should use PIMPL, but for making the classes for your game you should try to inline as much as possible to take advantage of compiler optimizations.

The whole "always define functions in CPP file" can also backfire: many people who try to learn templates often end up making a templated class and defining the function in the source file, which is wrong - it will not work. Templates must be defined inline, or their definition must be visible wherever they are used.


You can feel free to dispute or disagree with my points, but they're from my personal experience and readings, and they're working very well for me. Obviously they don't apply 100% of the time, but they usually do.

[ebonygeek45]

@ AbstractionAnon

I told myself to stop copying and pasting I don't know how but that could be the only reason for line 22. Thank you....

@ L B
????????

I thought getters and setters to protect the private data was supposed to be a good thing. That is the reason I have been making sure to do it. Isn't it standard?

[LB]

struct MyClass
{
    int health, age;
};

//...

MyClass mc;
mc.health = 10;
mg.age = 1;
//...
mc.health = mc.health-attackstrength*damageratio+defense*multiplier;

class MyClass
{
    int health, age;
public:
    int getHealth(){ return health; }
    int getAge(){ return age; }
    void setHealth(int h){ health = h; }
    void setAge(int a){ age = a; }
};

//...

MyClass mc;
mc.setHealth(10);
mg.setAge(1);
//...
mc.setHealth(mc.getHealth()-attackstrength*damageratio+defense*multiplier);

I do not see a difference between code that uses getters and setters and code that does not.

In both situations, encapsulation is broken, ruining good OO design.

[ebonygeek45]

I guess that the way I am doing it is more coding and typing...but I looked around the I-net and they say it is worth the extra work. I don't know....
I don't really see anything wrong with it.

Thank you everyone I am not sure I am not missing anything, but I am not stuck anymore which is awesome.

[LB]

What's wrong with it is that it is pointless as it still goes against encapsulation and proper OO class design. I'm not expecting you to understand instantly, but you will see why later.

[AbstractionAnon]

I don't really see anything wrong with it.

Consider the following. If your getters and setters are public (which they almost always are), they can be called from anywhere in the program. You might as well make the variables public. What have you gained by creating getters and setters?
You certained have not enhanced data encapsulation.

If you find yourself creating getters and setters, you need to step back and look at what you class does. Does the class actually "do" anything? Or is it just a container for some variables that are modified from all over the place? If the class has explicit actions, then it is those actions that should be changing the state of the class' variables.

[Lynx876]

Thank you Anon!!

I actually searched around and found little information. Even made a thread on here.

You just summed it up as I thought it should be! (: