Class can't access my vector

[mmillar]

Still very new to c++ and programming in general. Although I've been picking it up quickly. However, I've hit a roadblock. I created a vector to store my class data. The destructor of the class seems to work, but it does not clear data from the vector. I've tried doing this a few different ways but all have failed. In the code below, I'm attempted to access my global variable, which is located in bunny_sim.cpp, from my class in bunny.h. This is giving an undeclared identifier error. I've tried moving the vector into the header file, which ends up exponentially increasing my errors so I've reverted it back.

bunny_sim.cpp:

  #include <iostream>
#include <vector>
#include <cstdlib>
#include <time.h>
#include <string>
#include "bunny.h"

std::vector<std::string> maleBunnyNames = { "Jelly Bean", "Snowball", "Peanut", "Snoop", "Thumper", "Oliver", "Comet", "Stuart", "Midnight", "Billy", "Caramel", "Freddie", "Pepper", "Riley", "Hopper", "Pancake", "Spooky", "Chip", "Dusty", "Jesse" };
std::vector<std::string> femaleBunnyNames = { "Willow", "Peaches", "Sprinkles", "Sunny", "Bon Bon", "Maggie", "Snowy", "Lilly", "Flower", "Sugar", "Angel", "Sweet Pea", "Snowflake", "Licorice", "Luna", "Daisy", "Nala", "Gertie", "Milly", "Cookie" };
std::vector<std::string> bunnyColors = { "white", "brown", "black", "spotted" };
std::vector<Bunny> bunnyList;

// Program Start
int main() {
	srand (time(NULL)); // Initializes random time
	

	for (int i = 0; i < 5; i++) { // Creates first 5 bunnies
		int bunnyId = i;
		int sexNum = rand() % 2 + 1; // Random sex number
		int mutantPercent = 2; // mutant chance percent
		int mutantRoll = rand() % 100 + 1; // random roll for mutant
		bool isMutant = false;
		std::string randomName;
		std::string randomColor = bunnyColors[rand() % 4];
		std::string sex;
		if (sexNum == 1) { // sets male if 1 & sets name
			sex = "male";
			randomName = maleBunnyNames[rand() % 20];
		}
		else { // else sets female & sets name
			sex = "female"; 
			randomName = femaleBunnyNames[rand() % 20];
		}
		if (mutantRoll <= mutantPercent) { // Checks if roll is equal or below percentage
			isMutant = true;
		}
		Bunny bun(bunnyId, randomName, sex, randomColor, isMutant);  // Creates instance of bunny
		bunnyList.push_back(bun); // Adds bunny to vector
	}

	for (int i = 0; i < bunnyList.size(); i++) { // For Test purposes only
		std::cout << bunnyList[i].getId() << ", ";
		std::cout << bunnyList[i].getName() << ", ";
		std::cout << bunnyList[i].getSex() << ", ";
		std::cout << bunnyList[i].getAge() << ", ";
		std::cout << bunnyList[i].getColor() << ", ";
		std::cout << bunnyList[i].isMutant() << ", ";
		std::cout << '\n';
	}


	int temp1 = 0;
	do {
		for (int i = 0; i < bunnyList.size(); i++) {
			
			bunnyList[i].updateAge();
			std::cout << bunnyList[i].getName() << " is " << bunnyList[i].getAge() << '\n';
		}
		std::cout << '\n' << bunnyList.size() << '\n';
		temp1++;
	} while (temp1 < 20);

	int temp;
	std::cin >> temp;

	return 0;
}

bunny.h:

#ifndef BUNNY_H
#define BUNNY_H
#include <iostream>
#include <string>


class Bunny
{
	int id;
	std::string name;
	std::string sex;
	std::string color;
	int age;
	bool radioactive_mutant_vampire_bunny;
	
public:
	Bunny(int aId, std::string aName, std::string aSex, std::string aColor, bool rmvb = false) {
		id = aId;
		name = aName;
		sex = aSex;
		color = aColor;
		age = 0;
		radioactive_mutant_vampire_bunny = rmvb;

	}
	~Bunny() { std::cout << name << " had died.\n"; }

	int getId() { return id; }

	std::string getName() {return name;}

	std::string getSex() {return sex;}

	int getAge() {return age;}

	std::string getColor() {return color;}

	bool isMutant() {return radioactive_mutant_vampire_bunny;}

	void updateAge() {		
		if (age > 10) {
			Bunny::~Bunny();
			bunnyList.erase(bunnyList.begin() + (id - 1));
		}
		else {
			age++;
		}
	}
};

#endif 

[Repeater]

Bunny::~Bunny();
No no no. You should not be calling the destructor. Ever. It gets called for you at the right time.

Your bunny objects should not be trying to remove themselves from the vector. They shouldn't know or care what kind of container they are in. They shouldn't know anything about it.

[Ganado]

Welcome!

A few things. First, next time it would be helpful if you posted the error messages you get verbatim. This could make things easier to diagnose without having to copy and load your code.

Second, a destructor should only be called manually under extraordinary circumstances. The purpose of a destructor is to be called automatically after the lifetime of that object ends. In your case, the element in your bunnyList is destroyed when you call erase on line 43, so attempting to call the bunny destructor here is not only wrong, but would be redundant even if it worked.

In other words, remove line 42 from bunny.h

Next,

bunny.h: In member function 'void Bunny::updateAge()':
bunny.h:44:4: error: 'bunnyList' was not declared in this scope
    bunnyList.erase(bunnyList.begin() + (id - 1));
    ^~~~~~~~~

Indeed, a single Bunny object has no idea that there is a list of bunnies somewhere else. I do not suggest this design.

I suggest a different design: Your logic in main should handle erasing from the bunnyList when a bunny dies.

for example, in bunny.h, just have

	void updateAge() {		
			age++;
	}

And then in main,

		for (int i = 0; i < bunnyList.size(); i++) {
			
			bunnyList[i].updateAge();
			if (bunnyList[i].getAge() > 10)
			{
				bunnyList.erase(bunnyList.begin() + i);
			}
			else
			{
				std::cout << bunnyList[i].getName() << " is " << bunnyList[i].getAge() << '\n';
			}
		}

Alternatively, if you want to keep the logic for when a bunny is dead within the bunny class itself, I suggest an approach like:

			bunnyList[i].updateAge();
			if (bunnyList[i].isDead()) // add a new member function to your Bunny class
			{
				bunnyList.erase(bunnyList.begin() + i);
			}
			else
			{
				std::cout << bunnyList[i].getName() << " is " << bunnyList[i].getAge() << '\n';
			}

in bunny.h:

bool isDead() { return age > 10; }

Of course, there's more than one way to skin a cat-- I mean kill a bunny, but hopefully that makes things a bit more clear.

[Shibitto]

Use using std::string; in the header under includes, instead of Std::string name it will convolute your code, just until you can visualize what's happening, it's optional.

A destructor is allocated be default if not called apon, if not used with care it can really destroy and mess up your program.

Your having errors because your applying bad habits, you should be seperating it into a header system for classes, and a file system for logic or even test conditions and so forth, linking back to the main page. There's no limit to how many bunny file.cpp you would want but your best bet is to re-edit your entire source into a readable output.

Class bunny is in its own file system there for the main.cpp has no idea where it is, being out of scope. Your relying to heavily on bunny.h to make requests to absolutely nothing.

Your better of typing your code into one body of document rather seperating it into its own file system.

You should always simply your code before going down the rabbit hole.

#include <iostream>
#include <vector>
#include <cstdlib>
#include <time.h>
#include <string>
//#include "bunny.h"
using std::string;
using std::vector;
using std::cin;
using std::cout;
using std::endl;


class Bunny
{
	int id;
	string name;
	string sex;
	string color;
	int age;
	bool radioactive_mutant_vampire_bunny;
	
public:
Bunny(int aId, string aName, std::string aSex,string aColor, bool rmvb = false) {
		id = aId;
		name = aName;
		sex = aSex;
		color = aColor;
		age = 0;
		radioactive_mutant_vampire_bunny = rmvb;

	}
	~Bunny() { cout << name << " had died.\n"; }

	int getId() { return id; }

	string getName() {return name;}

	string getSex() {return sex;}

	int getAge() {return age;}

	string getColor() {return color;}

	bool isMutant() {return radioactive_mutant_vampire_bunny;}

	void updateAge() {		
		if (age > 10) {
	          age++;
		}
	}
};


vector<string> maleBunnyNames = { "Jelly Bean", "Snowball", "Peanut", 
"Snoop", "Thumper", "Oliver", "Comet", "Stuart", "Midnight", "Billy",
 "Caramel", "Freddie", "Pepper", "Riley", "Hopper", 
"Pancake", "Spooky", "Chip", "Dusty", "Jesse" };

vector <string> femaleBunnyNames = { "Willow", 
"Peaches", "Sprinkles", 
"Sunny", "Bon Bon", 
"Maggie",
 "Snowy", "Lilly", "Flower", "Sugar", 
"Angel", "Sweet Pea",
 "Snowflake", "Licorice", "Luna", 
"Daisy", "Nala", "Gertie", "Milly", "Cookie" };

vector<string> bunnyColors = { "white", "brown", "black", "spotted" };
vector<Bunny> bunnyList;

// Program Start
int main() {
	srand (time(NULL)); // Initializes random time
	

	for (int i = 0; i < 5; i++) { // Creates first 5 bunnies
		int bunnyId = i;
		int sexNum = rand() % 2 + 1; // Random sex number
		int mutantPercent = 2; // mutant chance percent
		int mutantRoll = rand() % 100 + 1; // random roll for mutant
		bool isMutant = false;
		std::string randomName;
		std::string randomColor = bunnyColors[rand() % 4];
		std::string sex;
		if (sexNum == 1) { // sets male if 1 & sets name
			sex = "male";
			randomName = maleBunnyNames[rand() % 20];
		}
		else { // else sets female & sets name
			sex = "female"; 
			randomName = femaleBunnyNames[rand() % 20];
		}
		if (mutantRoll <= mutantPercent) { // Checks if roll is equal or below percentage
			isMutant = true;
		}
		Bunny bun(bunnyId, randomName, sex, randomColor, isMutant);  // Creates instance of bunny
		bunnyList.push_back(bun); // Adds bunny to vector
	}

	for (int i = 0; i < bunnyList.size(); i++) { // For Test purposes only
		cout << bunnyList[i].getId() << ", ";
		cout << bunnyList[i].getName() << ", ";
		cout << bunnyList[i].getSex() << ", ";
		cout << bunnyList[i].getAge() << ", ";
		cout << bunnyList[i].getColor() << ", ";
		cout << bunnyList[i].isMutant() << ", ";
		cout << '\n';
	}


	int temp1 = 0;
	do {
		for (int i = 0; i < bunnyList.size(); i++) {
			
			bunnyList[i].updateAge();
			cout << bunnyList[i].getName() << " is " << bunnyList[i].getAge() << '\n';
		}
		cout << '\n' << bunnyList.size() << '\n';
		temp1++;
	} while (temp1 < 20);

	int temp;
	cin >> temp;

	return 0;
}

[mmillar]

Thank you everyone for your help. All of your comments helped me. I removed the call for the destructor. I thought I had to call it manually, and I moved the logic for deleting from the array to my main source fill and it solved the problem.

After doing that I noticed another weird glitch where it would state the same name died multiple times, and that all 5 bunnies weren't dying at the same time. I came to the realization that as the for loop counted up, the vector size got shorter, moving the indexes to the left for each item deleted. As such, not all items were getting deleted at the right times. I fixed it by reversing the for loop as such:

for (int i = bunnyList.size()-1; i >= 0; i--) {
			bunnyList[i].updateAge();
			if (bunnyList[i].getAge() > 10) {
				bunnyList.erase(bunnyList.begin() + i);
			}
			else {
				std::cout << bunnyList[i].getName() << " is " << bunnyList[i].getAge() << '\n';
			}
		}

[Ganado]

Good catch, that totally slipped my mind...

Here's an SO post that also discusses the issue of deleting while iterating.
https://stackoverflow.com/a/4713588/8690169

[mmillar]

Unrelated to that, I noticed another weird glitch when the program starts, and I haven't the foggiest as to why its happening. The console outputs the following:

// starting here is before the loop begins
Peaches has been born.
Peaches has died.

Luna has been born.
Peaches has died.

Luna has died.

Snowflake has been born.
Peaches has died.

Luna had died.

Snowflake has died.

Chip has been born.
Peaches had died.

Luna had died.

Snowflake had died.

Chip had died.

Bon Bon has been born.
Peaches had died.

Luna had died.

Snowflake had died.

Chip had died.

Bon Bon had died.
// Loop beings now
// Then displays all the class details and counts up age 

[Repeater]

What is it doing that you don't think it should, or what does it not do that you think it should be?

[mmillar]

Well, I think it should state when each bunny has been born, but should not indicate that they have died before they reach the age of 10. In the example above, its before the aging process has begun. So its like its calling the constructor once, and the destructor multiple times before the loop begins to start aging them.

[Ganado]

You're referring to how it says they died right after they're born, right?

This is because you're printing text in the destructor.


Lines 38 and 39 (of your original post): First you make a Bunny object called bun. Then you call push_back. Calling push_back makes a copy of the the Bunny object and adds it to the vector. But we're still left with the original bun object. This goes out of scope after the after line 39 (at the end of the scope the object was declared in).

Then, the next problem is that when a vector resizes after reaching its capacity, the vector has to copy its elements from the old internal array to the new one. Doing this will internally cause objects to be copied and destroyed. You aren't going to be able to easily prevent the destructors from running.

One way around this is to use emplace_back instead of push_back. This will stop some of the temporary objects from being made, but won't stop the destructors from being called when the vector re-allocates.

Honestly, you should avoid putting user-facing side effects in the destructor in this way. Destructors are more about doing background cleaning up of resources. Usually their side-effects shouldn't be something that the user of the class has to worry about.

I would move the printing to the updateAge part of the code, as a start.

[Repeater]

You get told a Bunny has died every time one is destructed.

	Bunny bun(bunnyId, randomName, sex, randomColor, isMutant);  // Creates instance of bunny
		bunnyList.push_back(bun); // Adds bunny to vector
}

See that } ? At that point, the Bunny object named bun is destroyed. There is a copy of that Bunny object, a whole new Bunny object, in the vector, but the Bunny object named bun is destroyed here.

I expect you have many other cases of Bunny objects being created and destroyed. For example, the vector manages its own size and memory; if it has to increase in size and occupy different memory, the objects in the vector must be copied intot he new (bigger) vector, and the old objects will then be destroyed.