Help With Pointer Arithmatic

I'm trying to get a program to run here that will fill a dynamic array of class objects using pointers. From what I can tell from the errors (Unhandled exception at 0x00EE59A6 in Lab6take2.exe: 0xC0000005: Access violation reading location 0x00000014.) I've done something wrong with the pointer labelled songlist, as it seems to be pointing to zero. Here's the relevant code.
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
MP3Database :: MP3Database(int _size){
	size = _size;
	MP3* songlist = new MP3[size];
}

void MP3Database::fillarray(){
	MP3* pointer = songlist;
	for (int i = 0; i < size; i++){
		string sentinal = "h";
		string song, artist;
		int stars, date;
		double length;
		cout << "Please input the name of the Song (has to be one word)" << endl;
		cin.ignore();
		getline (cin,song);
		song = "Songname";
		(*songlist).name = song;

		cout << "artist name (one word)" << endl;
		//getline(cin,artist);
		artist = "Artistname";
		songlist->artist = artist;

		cout << "Rating?" << endl;
		cin >> stars;
		while ((stars > 5) && (stars < 0)){
			cout << "Invalid rating, input a number between 0 and 5" << endl;
			cin >> stars;
		}
		songlist->setstars(stars);

		cout << "Length?" << endl;
		cin >> length;
		while (length < 0){
			cout << "Invalid length, input a number greater than 0." << endl;
			cin >> length;
		}
		songlist->length = length;

		cout << "Year?" << endl;
		cin >> date;
		while ((date > 2013) && (date < 0)){
			cout << "Invalid input, please input a number between 1 and 2013" << endl;
			cin >> date;
		}
		songlist->date = date;
		songlist++;
	}
}

cheers for any help lads
Last edited on
closed account (N36fSL3A)
Code tags please.
oops, there we are
In the constructor of MP3Database you declare a local variable named songlist.
That means that you allocate memory to a temporary variable, and not the "real" songlist member data.

1
2
3
4
5
MP3Database :: MP3Database(int _size){
	size = _size;
//	MP3* songlist = new MP3[size];
	songlist = new MP3[size];
}


So in the end the songlist member data was a bad pointer.

By the way, you didn't post the code for the ~MP3Database() destructor, so just in case you forgot:

1
2
3
4
MP3Database::~MP3Database()
{
    delete[] songlist; // important to delete[] what was new[]'d
}

Additionally, the way you use the dynamic array songlist is weird and can lead to errors.

46
47
48
49
50
51
		songlist->date = date;
// should use: songlist[x].date = date;

		songlist++;
// bad, you alter the songlist pointer...
// you must not in order to delete[] it correctly 
Last edited on
Cool, so that fixed that error. Now I'm getting something new when i try to use the pointer to modify elements within the dynamic array.

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

void MP3Database::fillarray(){
	for (int i = 0; i < size; i++){
		string sentinal = "h";
		string name, artist;
		int stars, date;
		double length;
		cout << "Please input the name of the Song" << endl;
		cin.ignore();
		getline (cin,name);
		songlist[i].name = name;
//Unhandled exception at 0x0FEF31CA (msvcr120d.dll) in Lab6take2.exe: 0xC0000005: Access violation writing location 0xABABABAB.
		cout << songlist[i].name;
		
		cout << "artist name" << endl;
		cin.ignore();
		getline(cin,artist);
		songlist[i].artist = artist;

		cout << "Rating?" << endl;
		cin >> stars;
		while ((stars > 5) && (stars < 0)){
			cout << "Invalid rating, input a number between 0 and 5" << endl;
			cin >> stars;
		}
		songlist->setstars(stars);

		cout << "Length?" << endl;
		cin >> length;
		while (length < 0){
			cout << "Invalid length, input a number greater than 0." << endl;
			cin >> length;
		}
		songlist[i].length = length;

		cout << "Year?" << endl;
		cin >> date;
		while ((date > 2013) && (date < 0)){
			cout << "Invalid input, please input a number between 1 and 2013" << endl;
			cin >> date;
		}
		songlist[i].date = date;
	}
}
The only problem that stands out to me is:

26
27
//		songlist->setstars(stars);
		songlist[i].setstars(stars);


Also, why do you use cin.ignore(); before calls to getline()?
the syntax
1
2
3
songlist->stars = stars;
//or
songlist->setstars(stars);

was what my lecturer recommended I do, and changing it to
 
songlist[i].setstars(stars);

doesn't seem to make any difference currently. The cin.ignore(); is the only thing that lets me enter text into the getline() function. Again, there doesnt seem to be any difference whether i use a getline() function or just regular old cin>> name;
At this point my suggestion would be that you post the full source code.
This is because the mistake (or mistakes) probably hide in the parts you did not yet post.
MP3.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
#include <string>
using namespace std;
#ifndef class_MP3
#define class_MP3
class MP3{

public:
	string name, artist;
	int stars, date;
	double length;

public:
	MP3();
	MP3(string n, string a, double l, int s, int d);
	string getname();
	void setname(string);

	string getartist();
	void setartist(string);

	double getlength();
	void setlength(double);

	int getstars();
	void setstars(int);

	int getdate();
	void setdate(int);

	void print();
};
#endif 


MP3.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
#include "MP3.h"
#include <string>
#include <iostream>

MP3 :: MP3(){
	MP3::setname("___");
	MP3::setartist("___");
	MP3::setlength(0);
	MP3::setstars(5);
	MP3::setdate(0);
}

MP3::MP3(string n, string a, double l, int s, int d){
	MP3::setname(n);
	MP3::setartist(a);
	MP3::setlength(l);
	MP3::setstars(s);
	MP3::setdate(d);
}

void MP3::setname(string n){
	name = n;
}
string MP3::getname(){
	return name;
}

void MP3::setartist(string a){
	artist = a;
}
string MP3::getartist(){
	return artist;
}
	
void MP3::setlength(double l){
	length = l;
}
double MP3::getlength(){
	return length;
}

void MP3::setstars(int s){
	stars = s;
}
int MP3::getstars(){
	return stars;
}

void MP3::setdate(int d){
	date = d;
}
int MP3::getdate(){
	return date;
}

void MP3::print(){
	cout << "Name: /t" << getname() << endl;
	cout << "Artist: /t" << getartist() << endl;
	cout << "Length: /t" << getlength() << endl;
	cout << "Stars: /t" << getstars() << endl;
	cout << "Released: /t" << getdate() << endl;
}


MP3Database.h

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

#include "MP3.h"
#ifndef class_MP3Database
#define class_MP3Database

class MP3Database{
public:
	int size;
	MP3* songlist = new MP3[size];

	//MP3Database();		will not be implemented in the scope of the program
	MP3Database(int);
	void print();
	void search(string);
	void fillarray();
	~MP3Database();
};
#endif


MP3Database.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
#include "MP3.h"
#include <string>
#include <iostream>
#include "MP3Database.h"

//MP3Database :: MP3database(){}

MP3Database :: MP3Database(int _size){
	size = _size;
	songlist = new MP3[size];
}

void MP3Database::print(){
	for (int i = 0; i < size; i++){
		cout << "MP3 Number: " << i << endl;
		cout << "Name: " << (*songlist).name << endl;
		cout << "Artist: " << songlist->getartist() << endl;
		cout << "Length: " << songlist->getlength() << endl;
		cout << "Rating: " << songlist->getstars() << endl;
		cout << "Release Date: " << songlist->getdate() << endl << endl;
		songlist++;
	}
	cout << "End of Library." << endl;

}

void MP3Database::search(string _artist){
	bool results = false;
	for (int i = 0; i < size; i++){
		
		if (_artist == songlist->artist){
			cout << "Match found: " << songlist->name;
			results = true;
		}
		songlist++;
	}
	if (results == false){
		cout << "Artist not found. Please recheck spelling and try again." << endl;
	}
}

MP3Database :: ~MP3Database(){
	delete[]  songlist;
}

void MP3Database::fillarray(){
	for (int i = 0; i < size; i++){
		string sentinal = "h";
		string name, artist;
		int stars, date;
		double length;
		cout << "Please input the name of the Song" << endl;
		cin>> name;
		songlist[i].name = name;
		cout << songlist[i].name;
		
		cout << "artist name" << endl;
		cin.ignore();
		getline(cin,artist);
		songlist[i].artist = artist;

		cout << "Rating?" << endl;
		cin >> stars;
		while ((stars > 5) && (stars < 0)){
			cout << "Invalid rating, input a number between 0 and 5" << endl;
			cin >> stars;
		}
		songlist[i].stars;

		cout << "Length?" << endl;
		cin >> length;
		while (length < 0){
			cout << "Invalid length, input a number greater than 0." << endl;
			cin >> length;
		}
		songlist[i].length = length;

		cout << "Year?" << endl;
		cin >> date;
		while ((date > 2013) && (date < 0)){
			cout << "Invalid input, please input a number between 1 and 2013" << endl;
			cin >> date;
		}
		songlist[i].date = date;
	}
}


Lab6.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
#include <iostream>
#include <iomanip>
#include "MP3Database.h"
#include "MP3.h"

using namespace std;
void fillarray(MP3Database, int);
void main(){
	int size = 0;
	cout << "Welcome to this (currently) pretty useless piece of software" << endl;
	cout << "To begin, please enter the size of your music collection;" << endl;
	cin >> size;
	MP3Database collection(size);
	collection.print();
	collection.fillarray();
	collection.print();
	cout << "Now that your music collection is entered, pleas enter the artist name you wish to search for." << endl;
	string artist;
	getline (cin, artist);
	cout << "Searching..." << endl;
	collection.search(artist);
	cout << "Search complete. Please input another artist." << endl;
	getline(cin, artist);
	cout << "Searching..." << endl;
	collection.search(artist);
	cout << "Search complete. Goodbye." << endl;
	system("PAUSE");
}


I was originally planning for the variables to be private, but I wrongly thought I was using the methods wrong, so i changed them to public.
Copy-pasting the code which is problematic, below.

In these functions you use pointer arithmetic on the songlist member data.
By doing this, you alter the songlist member data, and it eventually turns into a bad pointer, hence the crashes.

So you must rewrite the for() loops to use songlist[i].??? as you did in the case of MP3Database::fillArray().

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
void MP3Database::print(){
	for (int i = 0; i < size; i++){
		cout << "MP3 Number: " << i << endl;
		cout << "Name: " << (*songlist).name << endl;
		cout << "Artist: " << songlist->getartist() << endl;
		cout << "Length: " << songlist->getlength() << endl;
		cout << "Rating: " << songlist->getstars() << endl;
		cout << "Release Date: " << songlist->getdate() << endl << endl;
		songlist++;
	}
	cout << "End of Library." << endl;

}

void MP3Database::search(string _artist){
	bool results = false;
	for (int i = 0; i < size; i++){
		
		if (_artist == songlist->artist){
			cout << "Match found: " << songlist->name;
			results = true;
		}
		songlist++;
	}
	if (results == false){
		cout << "Artist not found. Please recheck spelling and try again." << endl;
	}
}


A minor issue:

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
#include "MP3.h"
#ifndef class_MP3Database
#define class_MP3Database

class MP3Database{
public:
	int size;

	// MP3* songlist = new MP3[size]; // huh?
	// does this not give at least a Warning? may be a C++11 feature though

	MP3* songlist;

	//MP3Database();		will not be implemented in the scope of the program
	MP3Database(int);
	void print();
	void search(string);
	void fillarray();
	~MP3Database();
};
#endif 
Alternatively: if your professor requires that you do pointer arithmetic, use a temporary pointer instead of songlist, as in:

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
void MP3Database::print(){
	MP3* tmp = songlist;

	for (int i = 0; i < size; i++){
		cout << "MP3 Number: " << i << endl;
		cout << "Name: " << tmp->name << endl;
		cout << "Artist: " << tmp->getartist() << endl;
		cout << "Length: " << tmp->getlength() << endl;
		cout << "Rating: " << tmp->getstars() << endl;
		cout << "Release Date: " << tmp->getdate() << endl << endl;
		tmp++;
	}
	cout << "End of Library." << endl;

}


This is still a clumsy approach though.
I can only get max 70% without it, so its a necessary evil, i'm afraid. But on a more positive note, that works perfectly now, so cheers!
But on a more positive note, that works perfectly now, so cheers!

Nice, but I hope you did not forget to make the corrections to MP3Database::search() as well.

If you want to refine your code a little bit, read these:
http://www.parashift.com/c++-faq/using-namespace-std.html
http://www.parashift.com/c++-faq/const-member-fns.html
http://www.parashift.com/c++-faq/istreams-remember-bad-state.html
Topic archived. No new replies allowed.