Does this program look effective?

[Outlaw782]

I am trying to develop an Address Book program, and I am wanting to do it the correct way. I feel most things are correct. I am using getters and setters here, however I feel like I'm not using the effectively. I feel like they're just sitting there haha. For example, in my for loop that prints out my vector of strings below, it effectively prints out my vector of strings, but shouldn't I be printing things out through my getter? I don't know, if you guys wouldn't mind to just critique it a little that would help me out so much! Thanks!

#include <iostream>
#include <vector>
#include <string>
#include <fstream>
using namespace std;

class Person
{
private:

	vector<string> this_name;
	vector<int> this_number;
	vector<string> this_address;

public:

	// getters

	vector<string> get_Name() const      //const  states that the method will not change any value within the class.
	{
		return this_name;
	}
	vector<string> get_Address()  const
	{
		return this_address;
	}
	vector<int> get_PhoneNumber() const 
	{
		return this_number;
	}

	// setters

	void set_Name(vector<string> N)
	{ 
		this_name = N; 

	}
	void set_Address(vector<string> A)
	{
		this_address = A;
	}
	void set_PhoneNumber(vector<int> P)
	{
		this_number = P;
	}
};



int main()
{
	
	Person Name;
	Person Address;
	Person PhoneNumer;
	string x;
	vector<string> all_addresses;
	int Answer;
	int z;

	ifstream datain;
	ofstream dataout;

	dataout.open("Address Book.dat");

	cout << "Please type 1 to enter addresses or 0 to exit: ";
	cin >> Answer;

	while (Answer == 1)
	{
		int v = 0;
		cout << "How many addresses would you like to enter?: ";
		cin >> z;
		cin.ignore();

		while (v < z)
		{
	
			cout << "Enter 1 address: ";
			getline(cin, x);


			all_addresses.push_back(x); // takes each string and pushes it to the end of the vector. This is how we make our vector of strings
		    Address.set_Address(all_addresses); // sets your vector of addresses to the set_Address setter.

			v++;
		}

		cout << "Please type 1 to enter an address or 0 to exit: ";
		cin >> Answer;
	}
		for (string a_address : all_addresses)
		{
			dataout << a_address << endl;
			cout << a_address << endl;
		}

	

	if (Answer == 0)
	{
		cout << "Program Terminating...";
		system("Pause");
		return 0;
	}
	
	system("Pause");
	return 0;
}

[booradley60]

I don't think you have a good grasp of expressing your problem in an object-oriented way. For example, this part

class Person
{
private:

	vector<string> this_name;
	vector<int> this_number;
	vector<string> this_address;
//... rest of code 

would seem to indicate that each person you would create would have a collection of names, a collection of numbers, and a collection of addresses. While I could see where a person could have multiple names, numbers, and addresses, is that really something you want to capture in this program? I would start off with having each person has only one name, one number, and one address.

And this part in main:

int main()
{
	
	Person Name;
	Person Address;
	Person PhoneNumer;
//... 

Is Name a person? Is Address a person? Is PhoneNumber a person? The way you've declared them here, then yes they are. But this object model doesn't seem to fit your problem. I would expect something more like:

int main()
{
	
	Person alice;
	Person marty;
	Person dave;
	//... 

Then you can use your setters (or, better yet, use a constructor) to initialize each person's full name, phone number, and address.

[Outlaw782]

Thanks for the response! The reason I had

class Person
{
private:

	vector<string> this_name;
	vector<int> this_number;
	vector<string> this_address;

was because I wanted the user to be able to enter multiple addresses at once so they didn't have to enter them one at a time, so I would store those multiple addresses, names, and numbers into a string of vectors. Would that be a bad idea?

And what you were talking about in in main seems to make sense, I will work on changing that. It's just a lot to take in at once when you're an amateur programmer :D

[booradley60]

I think you should add things one Person at a time. So get the name, number, and address for a person. Then, create a new person object with that data. Then, add that new person object to a list of persons.

#include <iostream>
#include <string>
#include <vector>

class Person
{
private:
    std::string _name;
    int         _number;
    std::string _address;

public:
    //constructor that initializes all the private variables using parameters
    //supplied by whoever invokes this constructor
    Person(std::string newName, int newNumber, std::string newAddress)
        : _name(newName), _number(newNumber), _address(newAddress)
        { }
        
    //getters, setters would go down here
};

class AddressBook
{
private:
    std::vector<Person> _people;

public:
    void addPerson(const Person& p)
    {
        _people.push_back(p);
    }
};

int main()
{
    AddressBook myAddBook;
    
    const int numPeopleInBook = 5;
    for(int i = 0; i < numPeopleInBook; ++i)
    {
        std::string nameFromUser, addressFromUser;
        int numberFromUser;
        
        std::cout << "Give me a name: ";
        std::getline(std::cin, nameFromUser);
        
        std::cout << "Give me a number: ";
        std::cin >> numberFromUser;
        std::cin.ignore();
        
        std::cout << "Give me an address: ";
        std::getline(std::cin, addressFromUser);
        
        Person p(nameFromUser, numberFromUser, addressFromUser);
        myAddBook.addPerson(p);
    }
}

There are some tricks you can do here to save a few copies of objects made behind the scenes for you, but I wouldn't worry about those yet. This code simply gets data from the user to create a person, and adds that person to the AddressBook. It repeats 5 times. It at least demonstrates what it means when a Person "has-a" name/number/address, and an AddressBook "has-a" collection of people.

[Outlaw782]

Thank you for your help booardley60, I will work on this tonight. You were very helpful, is there any way I can give you positive rep or something?

[booradley60]

Rep around here is gained and lost through arguments in the Lounge. Don't worry about it. :)