Understanding Pointers

[LuckyXII]

TLDR? see below

I'm trying to learn programming but have a hard time understanding how to properly use pointers.

I have a pre-existing code that I've written, it's a simple program collecting transactions and calculating a pot and how much a person should add or remove from the pot in order to have spent an equal amount despite who paid for which.

The code is all written using static arrays and I'm now trying to use dynamic arrays only but I don't fully understand how and when to use assign/copy constructors and I have memory issues.

(the code is a mix of swedish and english but I hope my comments explains enough)

as an example:

-take the first class "Transaktion" and a bit of a class TransaktionsLista which collects transactions

-then a piece of main using the class(and others that are still static atm)

-and finally the acctual codes from methods and constructors etc

  //Transaction
class Transaktion {

private:
	std::string datum; //date
	std::string typ; //type
	std::string namn; //name
	double belopp; //amount
	int ant_kompisar; //#friends
	std::string *kompisar; //friends

public:
	Transaktion();
	~Transaktion();
	Transaktion(const Transaktion &T); //copy
	Transaktion& operator=(const Transaktion& t); //overl assign
	std::string haemta_namn(); //get name
	double haemta_belopp(); //get amount
	int haemta_ant_kompisar(); //get #friends
	bool finnsKompis(std::string namnet); //find friend
	bool laesEnTrans(std::istream &is); //read transaktion
	void skrivEnTrans(std::ostream &os); //print transaktion
	
===========================================================================
        int main() {

	std::string fileName;
	int choice = 0;//used later for menu
	//objects
	Transaktion T;
	TransaktionsLista TL; //TransactionList
	PersonLista PL; //PersonList

	//function checks fileName
	Namn_pa_fil(fileName); //looks for input file name etc

	//open file for read 
	std::ifstream IN(fileName);

	//check for error
	if (!IN) {
		std::cout << "File could not be opened" << std::endl;
		exit(EXIT_FAILURE);
	}

	//if no error read file
	TL.laesin(IN); //read multiple transactions and add to list

	//fix Persons
	PL = TL.FixaPersoner(TL); //add paid/debt to each person
        .................................
==============================================================================
code:

/Transaction
//----------------------------------------------------------------------------
//constructors

Transaktion::Transaktion()
	: datum("00000"), typ("Ingen"), namn("ingen"), belopp(0), ant_kompisar(0)

{
	kompisar = 0;
}

Transaktion::~Transaktion() {
	delete [] kompisar;
}


//--------------------------------------------
//Assign/copy

//copy
Transaktion::Transaktion(const Transaktion &T) {
	if (kompisar > 0) {
		kompisar = new std::string[ant_kompisar];

		for (int i = 0; i < ant_kompisar; i++) {
			kompisar[i] = T.kompisar[i];
		}
	}
	else
		kompisar = 0;
		std::cout << "NULL!" << std::endl;
}

//overload assign
Transaktion &Transaktion::operator=(const Transaktion &t) {
	if (this != &t) {

		delete[] kompisar;
		datum = t.datum;
		typ = t.typ;
		namn = t.namn;
		belopp = t.belopp;
		ant_kompisar = t.ant_kompisar;
		kompisar = new std::string[ant_kompisar];

		for (int i = 0; i < ant_kompisar; i++)
			kompisar[i] = t.kompisar[i];
	}

	if (this == 0) {
		std::cout << "This NULL" << std::endl;
	}
	return *this;
}
=============================================================================
//read file or from keyboard
bool Transaktion::laesEnTrans(std::istream &is) {

	int ak=0;
	std::string d, t, n,*k;
	double b=0;


	is >> d >> t >> n >> b >> ak;
	k = new std::string[ak];
	for (int i = 0; i < ak; i++) {
		is >> k[i];
		
	}

	datum = d;
	typ = t;
	namn = n;
	belopp = b;
	ant_kompisar = ak;
	kompisar = k;
	
	return !is.eof();
}

in the method for reading a file if I print the values each time the method is called by "TL.laesin(IN)" in main I can see that even though there are no more values left to get in my "filename.txt" file I'm still creating another empty transaktion which is probably the cause for a leak.

an example would be:
//last transaktion in file
date:160105 type:transport name:Johan amount:2000 #friends:3 friends:nr1 nr2 nr3
//empty transaktion is printed efter file ends
date: type: name: amount:0 #friends0//friends are not printed

this is what happens eventually:
Unhandled exception at 0x779AD8A8 in xxxxxxx: Microsoft C++ exception: std::bad_alloc at memory location 0x0018C6F0.

like I explained I lack basic understanding so I need to know what I've done wrong, sadly there is no one I can discuss programming with or get help from in a way I understand so I keep making mistakes without knowing why

TLDR:

I would like explained what I've done incorrectly and the proper way to use pointers, assign and copy constructor.

P.S!

As not one of my friends or their acquaintances knows a bit about programming and any course I do has to be done at a distance I have no one to discuss, collaborate and learn from so perhaps if you're near my level or above but seeking someone to "mentor" or learn side by side with I'd love to have someone to chatt with.

I do realize there are great opportunities at places such as github but I'm not comfortable with my level so it's hard to approach someone

sorry for the long post, so long and thanks for all the fish.

[Peter87]

You forgot to copy ant_kompisar (and a lot of other variables) in the copy constructor. Look at the copy assignment operator and you will see what is missing.

The copy constructor always prints "NULL!" because line 86 is not part of the if-else statement. If you want the else-part to contain more than one statement you need to use curly brackets { }.

It doesn't make sense to test if a pointer is greater than zero (it should give you an error). If you want to check if the pointer is not null you can do

if (kompisar)

or, more explicitly,

if (kompisar != nullptr)

If you prefer to use 0 as a null pointer it is possible to use that instead

if (kompisar != 0)

In the copy assignment operator you test if the this pointer is null. You can do this if you want but it's a bit unnecessary because it should not be possible to happen.

In the laesEnTrans function you forgot to check if the kompisar pointer is already pointing to something. In that case you need to first use delete[] before you assign the new pointer value of k.

the code is a mix of swedish and english but I hope my comments explains enough

I'm Swedish but I always write all my code in English. If you write your code in English it will be much easier to share your code with people around the world and you don't need to come up with substitute letters for å, ä and ö. At university (a Swedish one) we learned that all code should be written in English (even if the course was in Swedish). You can do what you want but I think you should at least be consistent.

[LuckyXII]

Thanks!

Checking if kompisar is already pointing to something, I know that's basic thinking when working with pointers but I have 0 experience so I keep missing these things all the time :/

The program was part of an assignment and some methods were supposed to be name specific...I don't know why they wanted swedish variables.

I never used the copy constructor in this class thankfully but I don't know why I missed every other variable in it either ^^

anyway I solved one issue where PersonLista TransaktionsLista::FixaPersoner(); wasn't working with the dynamic memory


Now I can use some of the faults as a guide converting the rest of the program, cheers!