Yet another linker error (class declarations included)

[GraemeW]

Im getting this error in my code

[Linker Error] Unresolved external 'Employee::Employee(_STL::basic_string<char, _STL::char_traits<char>, _STL::allocator<char> >, _STL::basic_string<char, _STL::char_traits<char>, _STL::allocator<char> >, bool, int)' referenced from C:\SECURITY\MAIN.OBJ

See Updated Code Below

I think the error is on the last line in my main code, "Vector.pushback( new Employee( S, F, Auth, ID ));"
I can't see a single error in those class declarations? Can anyone tell me whats wrong?

[Paul McKenzie]

I do not get the problem with the following code:
// Employee.h

#ifndef EMP_H
#define EMP_H

#include <string>
class Employee {
public:
Employee( std::string S, std::string F, bool Auth, int ID );
std::string Surname;
std::string Forename;
bool Authorised;
int IDNo;
};
#endif

Employee.cpp

#include "employee.h"

Employee::Employee( std::string S, std::string F, bool Auth, int ID ) // Default Constructor
{
Surname = S;
Forename = F;
Authorised = Auth;
IDNo = ID;
}

MAINProg.cpp

#include <vector>
#include <string>
#include "employee.h"

std::vector<Employee*> Vector;

int main()//int argc, char* argv[]
{
int choice, ID, maxID = 1;
std::string S, F;
bool Auth;
char yesno;

// You know this is an illegal access, right?
Vector[0] = new Employee( S, F, Auth, ID );
}

There are other problems with the code you posted. First, you should change your prototype to the following:

Employee( const std::string& S, const std::string& F, bool Auth, int ID );

This eliminates the copying that the compiler would do when you are passing the strings to your function.

The second problem (unless you left out code) is that you are attempting to set Vector[0] to a value. Your vector has no element 0, it doesn't have any elements. Therefore what you are doing will cause an access violation. Either size the vector appropriately before using it, or use push_back() to add an item to the vector. You can either size a vector by utilizing the vector's constructor that takes an int argument, or by calling resize().

I would also call it something else besides "Vector". "EmployeeList" or "EmployeeVector" is much more descriptive.

Vector.resize(1); // Makes sure that Vector has one element available

Regards,

Paul McKenzie

[GraemeW]

Originally posted by Paul McKenzie
There are other problems with the code you posted. First, you should change your prototype to the following:

Employee( const std::string& S, const std::string& F, bool Auth, int ID );

This eliminates the copying that the compiler would do when you are passing the strings to your function.


Hi Paul,

I followed your advice and changed my code to "pushback" and changed the name of the vector. I tried your suggestion of using const std::string etc etc.. but was presented with a few errors.

I would like to use this method of pass by reference but when you say prototype do you mean in main? The header file or the Employee.cpp file?

If you don't get this error can you suggest any reason why I might? I have tried making a new "project" but the errors still occur.

[PaulWendt]

Well, basically the linker is telling you that your symbol can't be
found. I'd make explicitly sure that you are actually linking with
the module that contains the symbol in question [in this case,
the constructor]. If your compiler's IDE has a log of builds, it'd
be a good idea to check.

I don't know if you're using a Borland compiler or not, but I
remember reading on the net a long time ago that some versions
of their compiler didn't work well with directories that had
embedded spaces in them. Try changing your directories to the
8.3 format and see if that helps. Granted, that's a shot in the
dark, but <shrug>.

--Paul

[GraemeW]

Code:
Main.cpp
#include <iostream.h>
#include <string>
#include <conio>
#include <vector>

#include "Employee.h"
#include "SwipeCard.h"
using std::string;

int main()//int argc, char* argv[]
{
int choice, ID, maxID = 1;
string S, F;
bool Auth;
char yesno;

std::vector<Employee*> EmployeeVector;

EmployeeVector.push_back(new Employee( S, F, Auth, ID ));

Employee.h
#ifndef EMPLOYEE_H
#define EMPLOYEE_H
class Employee {
public:

Employee( std::string S, std::string F, bool Auth, int ID );

void SwipeCard(); // The employee swipes his card

private:
string Surname;
string Forename;
bool Authorised;
int IDNo;
};
#endif //EMPLOYEE_H

Employee.cpp
#include <string>
#include "Employee.h"
using std::string;
Employee::Employee( std::string S, std::string F, bool Auth, int ID ) // Default Constructor
{
Surname = S;
Forename = F;
Authorised = Auth;
IDNo = ID;
}


A few changes have been made yet the same error still exists, I have moved the project folder to the root of C: and added pushback etc.

Any ideas gratefully recieved.

[PaulWendt]

I wouldn't know; the code looks valid if the locations are correct
and the Employee module is actually being linked. Perhaps it is
a problem with the compiler/linker or its setup. Why don't you
post the compiler you're using? I know from the error message
that I haven't used it, but perhaps another person here has.

--Paul

[Paul McKenzie]

Originally posted by GraemeW
Hi Paul,

I followed your advice and changed my code to "pushback" and changed the name of the vector. I tried your suggestion of using const std::string etc etc.. but was presented with a few errors.
What were the errors?
I would like to use this method of pass by reference but when you say prototype do you mean in main? The header file or the Employee.cpp file?I don't know what you mean by "in main()". The main() is just a function.

The prototype is where the function is declared. For class member functions, this is found within the class definition. In Employee.h, you've defined the constructor as taking std::string arguments. In your implementation in Employee.cpp, the constructor matches what is in Employee.h. When you changed this to const std::string&, did you do it so that the .h matches the .cpp? Is this what the error is?

//Employee.h
class Employee
{
public:
Employee(const std::string& s1, const std::string& s2,
bool b, char c);
};

// Employee.cpp
Employee::Employee(const std::string& s1, const std::string& s2,
bool b, char c)
{
//
}

Also, the correct standard header is <iostream>, not iostream.h

Another thing -- any reason why you need to create a vector<Employee*> instead of vector<Employee>? This latter will relieve you from having to use new Employee(...) and will make your code easier to handle since you will not have to be responsible for cleanup (calling delete) at the end.

Regards,

Paul McKenzie

[Paul McKenzie]

Originally posted by GraemeW
A few changes have been made yet the same error still exists, I have moved the project folder to the root of C: and added pushback etc.

Any ideas gratefully recieved. Put the entire thing in one file and compile / link. You will see that the problem is either with your project not compiling the correct files, or you have multiple versions of your cpp and h files floating around, and your compiler is not linking or compiling the correct version. Here is a single file that contains all of the relevant code that you say the linker can't find.

#include <iostream> // Not iostream.h
#include <string>
// #include <conio> // What is conio?
#include <vector>

class Employee {
public:

Employee( const std::string& S,
const std::string& F, bool Auth, int ID );

private:
std::string Surname;
std::string Forename;
bool Authorised;
int IDNo;
};

Employee::Employee(const std::string& S,
const std::string& F, bool Auth, int ID )
{
}

using namespace std;

int main()
{
int choice, ID, maxID = 1;
string S, F;
bool Auth;
char yesno;

Employee E(S, F, Auth, ID);
std::vector<Employee> EmployeeVector; //
EmployeeVector.push_back(E);
}

I made some changes, like included <iostream> and illustrated how to use vector<Employee>, but if this compiles and links, then the problem is your compiler or project set up. Make a brand new project and copy the entire code above to a single CPP file. Compile and link only this file.

Regards,

Paul McKenzie

[GraemeW]

Hi Paul,

I have made a new project and used that single file and it compiled and ran first time. Obviously I have some stray files running around, I did a search for all the files that I had created and deleted each file.

I have since created a fresh versions in a new directory and it still won't work. I might do all the code in one cpp file and leave it at that.

Before I start on making one big cpp file can I ask a few questions from your suggestions?

I have always been told to use #include <iostream.h> when I changed to <iostream> I recieved the following errors (when I do my new code I will try both ways):
[C++ Error] Main.cpp(20): E2451 Undefined symbol 'cout'
[C++ Error] Main.cpp(20): E2451 Undefined symbol 'endl'
[C++ Error] Main.cpp(33): E2451 Undefined symbol 'cin'
Which way do you think is best? And if I use <iostream> and get the errors mentioned above should I just change back to <iostream.h>?

any reason why you need to create a vector<Employee*> instead of vector<Employee>? This latter will relieve you from having to use new Employee(...) and will make your code easier to handle since you will not have to be responsible for cleanup (calling delete) at the end.

I just presumed that I should make a vector of pointers to Employee's. Each employee will in turn hold a pointer to a swipecard etc as well.

What does vector<Employee> rather than vector<Employee*>? Surely it is better to hold pointers to an Employee?


Also...
Originally posted by Paul McKenzie
// #include <conio> // What is conio?


Conio.h includes an operation called getch(), which I place at the end of main.cpp

When the compiler executes the file and it reaches that point it stops and waits, I use it to stop the console flashing on then off.

[PaulWendt]

If you #include <iostream> you must use a using directives as
the symbols the compiler can't find reside in the std namespace.
It's typical to either:

using std::cin;
using std::cout;
// etc

or

using namespace std;


Edit: Oh, if you're going to hold a vector of pointers, I suggest
you look at boost's shared_ptr. It's basically a smart pointer
which will enable you to easily have containers to hold
polymorphic types. You can check out the smart pointers: URL=http://www.boost.org/libs/smart_ptr/index.htm]here[/URL].

conio.h isn't a standard header file. If you're going to use it,
that's fine ... but I'd wrap the thing in an #ifdef so that your
code remains portable.

--Paul

[Paul McKenzie]

Originally posted by GraemeW
I have always been told to use #include <iostream.h>Whoever told you this is wrong. The iostream.h is a non-standard C++ header. The standard C++ header is <iostream>.
when I changed to <iostream> I recieved the following errors (when I do my new code I will try both ways):
[C++ Error] Main.cpp(20): E2451 Undefined symbol 'cout'
[C++ Error] Main.cpp(20): E2451 Undefined symbol 'endl'
[C++ Error] Main.cpp(33): E2451 Undefined symbol 'cin'

cout, cin, endl, are defined in the std namespace when you include <iostream>. For example,

std::cout << "Test" << std::endl;

or you specify "using namespace std;" before referring to cout, cin, etc.
What does vector<Employee> rather than vector<Employee*>? Surely it is better to hold pointers to an Employee?No, not necessarily, and most of the time, you don't store pointers in vectors. Let me ask you, why do you think it is better to hold pointers than objects?

The vector holds a copy of your object. Once the vector has a copy, there is no need to "hold onto" a pointer. You can refer to the data right out of the vector container. When you place a dynamically allocated pointer in a vector, you have the extra maintenance of cleaning this up. With the code you posted, you have to call delete for all of those dynamically allocated Employee classes -- with my code, no such code to delete has to be written. My method leads to less code maintenance and more bug-free programs. Your method still leads to the maintenance of a pointer.

Basically, a good C++ programmer tries to avoid unneccessary usage of dynamically allocated objects for these reasons. For every "new" you don't have to make, the safer and more stable the program becomes.

Having said this, there are legitimate reasons to make a vector of pointers. However, your Employee class doesn't really fit any of the reasons. Basically, the reasons are this

a) The object doesn't have a copy constructor or assignment operatr

b) The sizeof(object) is very large, so storing copies would be prohibitive.

c) The object does have a copy ctor and assignment, but those operations are expensive (in terms of time used and space).

d) The vector will be used to store base class pointers so that base class and derived class pointers can be contained in a single vector (mostly this is used for polymorphism).

Your Employee class fits none of the scenarios above. It doesn't fit d), since I don't see any intention of deriving from it to create other Employee types (your Employee class lacks a virtual destructor, so it really can't be derived from safely).
Conio.h includes an operation called getch(), which I place at the end of main.cppThen why did you say <conio> instead of <conio.h>?
When the compiler executes the file and it reaches that point it stops and waits, I use it to stop the console flashing on then off. A simple "cin >> dummy" would accomplish the same thing without introducing conio.h.

Regards,

Paul McKenzie

[GraemeW]

Right well ill change all the iostreams and conios.. but I still can't get past this linker problem.

I have tried deleteing the two files to do with Employee and then running, it says can't find blah blah so obivously it is pointing/linking to the right files, then when I put them back it says linker error.

I have tried making a constructor with no values and that doesn't work, I just can't get round it. This is only the beginning of a much bigger project and im getting nowhere...

Any ideas?

[Paul McKenzie]

Originally posted by GraemeW
Right well ill change all the iostreams and conios.. but I still can't get past this linker problem.

I have tried deleteing the two files to do with Employee and then running, it says can't find blah blah so obivously it is pointing/linking to the right files, then when I put them back it says linker error.

I have tried making a constructor with no values and that doesn't work, I just can't get round it. This is only the beginning of a much bigger project and im getting nowhere...

Any ideas? What compiler and version are you using?

Regards,

Paul McKenzie

[GraemeW]

Hi Paul

Its Borland C++ Builder 6. Can I contact you outside of this board or would you rather me post here?

Thanks

[Paul McKenzie]

Originally posted by GraemeW
Hi Paul

Its Borland C++ Builder 6. Can I contact you outside of this board or would you rather me post here?

Thanks Your in luck. I have that compiler. If the examples I posted previously do not work, I will see if I can get them to work on my compiler here.

Regards,

Paul McKenzie

[GraemeW]

Hi Paul,

I have just cut/pasted the classes into one cpp file and it has ran for the first time... ill continue to cut/paste the other classes (in total there will be approx 8) and see how it gets on..

Im also going to try your method of storing the employee's by value rather than reference.. ill let you know how I get on. Thanks for all the help I can only guess how annoying it must be when someone constantly asks questions.

THanks for holding out.