Advertisement

C++ Class Constructor

Started by December 27, 2015 09:09 PM
8 comments, last by Alberth 8 years, 11 months ago

Hello,

So I am working on a simple phonebook app using file reading/writing, but in my Phonebook class constructor I am having issues. For some reason it is not allowing me to pass a string argument to the constructor. I took out the argument in the definition and declaration and it compiled fine and ran so I know that is where the issue is.

The error message is "C2664 'Phonebook::Phonebook(Phonebook &&)': cannot convert argument 1 from 'std::string' to 'const Phonebook &' MyPhonebook c:\users\brandon\documents\visual studio 2015\projects\myphonebook\myphonebook\main.cpp 30" and it doesn't make any sense to me. I googled the error message and I am passing a string to the constructor not any other type. I also get another error message saying "C4430 missing type specifier - int assumed. Note: C++ does not support default-int MyPhonebook c:\users\brandon\documents\visual studio 2015\projects\myphonebook\myphonebook\person.h 14"

Can someone please help me see what is happening? I would greatly appreciate it!

Phonebook.h


#pragma once 

#include "LibIncludes.h"

class Phonebook
{
public:
	Phonebook(string tmp_pb_name);
	void addEntry();
	void deleteEntry();
	void searchByFirstName() const;
	void searchByNumber() const;
	void displayAllEntries() const;
	void clearAllEntries();
	int mainMenu();
private:
	string phonebook_name;
};

Phonebook.cpp


///////////////////////////////////////
//
//   Phonebook definitions   
//
///////////////////////////////////////

#include "LibIncludes.h"

Phonebook::Phonebook(string tmp_pb_name)
{
	phonebook_name = tmp_pb_name;
}

//--------------------------------------------------------------------

int Phonebook::mainMenu()
{
	int option = 0;

	do
	{
		system("CLS");

		cout << "\n\n\t\t-----" << phonebook_name << " Phonebook-----";
		cout << "\n\t\t|";
		cout << "\n\t\t|\t[1] Create New Entry";
		cout << "\n\t\t|\t[2] Delete Entry";
		cout << "\n\t\t|\t[3] Display ALL Entries";
		cout << "\n\t\t|\t[4] Search By FIRST Name";
		cout << "\n\t\t|\t[5] Search by PHONE NUMBER";
		cout << "\n\t\t|\t[6] Clear ALL Entries";
		cout << "\n\t\t|\t[7] Exit Your Phonebook";
		cout << "\n\t\t|----------------------------------------------";

		cout << "\n\n\t\t> ";
		cin >> option;

		if (option < 1 || option > 7)
		{
			cout << "\n\t\t*** [ Error! Option Out Of Range! ] ***\n";
			system("PAUSE");
		}
	} while (option < 1 || option > 7);

	return option;
}

//--------------------------------------------------------------------

void Phonebook::addEntry()
{
	
}

//--------------------------------------------------------------------

void Phonebook::displayAllEntries() const
{

}

//--------------------------------------------------------------------

void Phonebook::clearAllEntries()
{

}

//--------------------------------------------------------------------

void Phonebook::deleteEntry()
{

}

//--------------------------------------------------------------------

void Phonebook::searchByFirstName() const
{

}

//--------------------------------------------------------------------

void Phonebook::searchByNumber() const
{

}

Main.cpp


///////////////////////////////////////
//
//   Basic phonebook app that
//   uses writing and reading
//   from files for the data
//   entries
//
//////////////////////////////////////

#include "LibIncludes.h"

string setPhoneBookName()
{
	string name; 

	cout << "\n\n\n\n\t\t\t\t-----Please Enter In Your Desired Phonebook Name-----";
	cout << "\n\t\t\t\t> ";
	cin >> name;

	return name;
}

//---------------------------------------------------------------------------------------

int main()
{
	int user_option = 0;
	bool exit = false;

	Phonebook myBook(setPhoneBookName());

	while (user_option != 7)
	{
		while (!exit)
		{
			user_option = myBook.mainMenu();

			switch (user_option)
			{
			case 7:
				cout << "\n\n\t\t---------------Goodbye---------------\n";
				cout << "\t\t";
				system("PAUSE");
				exit = true;
				break;
			default:
				cout << "\n*** [ Invalid Option! ] ***\n";
			}
		}
	}

	return 0;
}

LibIncludes.h


/////////////////////////////////////
//
//   All header files needed
//
/////////////////////////////////////

#pragma once

#include <iostream>
#include <string>
#include <fstream>

#include "Person.h"
#include "Phonebook.h"

using namespace std;
It might be a bit late in my timezone bust have you considered you are falling victim to the most vexing parse?

Also, you should never get into the habit of any 'using namespace' in general and std in particular in a header. Typing an extra std:: is not bad, adds a lot of clarity and the first time your polluted global namespace gives you hell you will understand the remaining point.
Advertisement

I am not sure, but it looks like the compiler wants to move instead of copying.


Phonebook::Phonebook(const string & tmp_pb_name) :
	phonebook_name(tmp_pb_name)
{
}
Phonebook::Phonebook(string && tmp_pb_name) noexcept:
	phonebook_name(std::move(tmp_pb_name))
{
}

The first thing, that is a bit interesting: that you can initialize the phonebook_name variable like me in this code.

It is a bit faster, than yours.

In your code is called default ctor, and the assigment later.

In my is only ctor with value to copy.

The first version copies string from the parameter, the second moves.

(the source become invalid or empty after the movement, for example string becomes an empty one, but it is more efficient sometimes)

P. S.

In your code you pass string by value, sometimes is reference is better.

You have an error in person.h, which is included before phonebook.h. It's possible that that error, which sounds like it may be a typo or a syntax error but is in a source file you did not include in your post, may be causing a cascade of interpretation errors elsewhere in the translation unit. Try focusing on that problem first.

Also, to eliminate the possibility of the most vexing parse, try changing line 30 of phonebook.cpp to


Phonebook myBook = PhoneBook(setPhoneBookName());

and see what happens.

Stephen M. Webb
Professional Free Software Developer


Also, you should never get into the habit of any 'using namespace' in general and std in particular in a header. Typing an extra std:: is not bad, adds a lot of clarity and the first time your polluted global namespace gives you hell you will understand the remaining point.

I have no idea why, but I converted everything to 'std::' and removed the using namespace and that solved my issue! So weird how it was that tat was causing trouble

Thanks to everyone who tried to help me as well! I researched most vexing parse and I learned something new!

Kubera has a right - your compiler tries to use move constructor for your class. setPhoneBookName() returns temorary string object (aka rvalue), and compiler can not use your constructor in this case.

You can change your code to store value of the setPhoneBookName() in new instance and call your constructor with it:

string myname = setPhoneBookName();

Phonebook myBook(myname);

alternative is to create move constructor as kubera sugested in one of the previous response.

regarding your second error C4430 - this is usally happen when return type is missing or it is not recognized. If you post your code I can give you more hints.

Advertisement
Sorry, mligor, but that is pretty much nonsense from start to finish. The return value of setPhoneBookName() is a temporary but that can bind fine to a 'const std::string&' (to copy-construct the std::string parameter) or directly to a 'const std::string&' parameter (which would have been a good idea to do). Most compilers will not even need to do that because they can just RVO a copy away.

Storing the return value in a local variable first is not needed. Not pre-C++11, not after. If that helps in your scenario you are just changing the symptoms of a more severe underlying problem.
Did you try to remove include person.h? I don't know what is written inside.

Did you try to remove include person.h? I don't know what is written inside.

I just changed all the items in the std namespace to have the prefix std:: and that for some reason solved everything. Weird, there was no issues in Person.h, but that std:: issue caused other things to show up as an error

std namespace is very big, it may have added names for things you don't know to exist :)

(nor do I by the way, which is why I also always use the std:: prefix, at least I then know what I get :) )

This topic is closed to new replies.

Advertisement