views:

167

answers:

6

Hi. I am teaching myself to write classes in C++ but can't seem to get the compilation to go through. If you can help me figure out not just how, but why, it would be greatly appreciated. Thank you in advance! Here are my three files:

make_pmt.C

#include <iostream>
#include "pmt.h"

using namespace std;


int main() {
    CPMT *pmt = new CPMT;
    pmt->SetVoltage(900);
    pmt->SetGain(2e6);

    double voltage = pmt->GetVoltage();
    double gain= pmt->GetGain();

    cout << "The voltage is " << voltage
         << " and the gain is " << gain << "." <<endl;

    return 0;
}

pmt.C

#include "pmt.h"

using namespace std;

class CPMT {
    double gain, voltage;
    public:
        double GetGain() {return gain;}
        double GetVoltage() {return voltage;}

        void SetGain(double g) {gain=g;}
        void SetVoltage(double v) {voltage=v;}
};

pmt.h

#ifndef PMT_H
#define PMT_H 1

using namespace std;

class CPMT {
    double gain, voltage;
    public:
        double GetGain();
        double GetVoltage();

        void SetGain(double g);
        void SetVoltage(double v);
};

#endif

And for reference, I get a linker error (right?):

Undefined symbols:
  "CPMT::GetVoltage()", referenced from:
      _main in ccoYuMbH.o
  "CPMT::GetGain()", referenced from:
      _main in ccoYuMbH.o
  "CPMT::SetVoltage(double)", referenced from:
      _main in ccoYuMbH.o
  "CPMT::SetGain(double)", referenced from:
      _main in ccoYuMbH.o
ld: symbol(s) not found
collect2: ld returned 1 exit status
+4  A: 

The syntax of your pmt.C file is wrong. It should read

double CPMT::GetGain() {return gain;}
double CPMT::GetVoltage() {return voltage;}
void CPMT::SetGain(double g) {gain=g;}
void CPMT::SetVoltage(double v) {voltage=v;}
Gareth Stockwell
+1  A: 

Try renaming the files .cpp or .cxx, your compiler may be assuming that .C means it is C and not C++, which seems to be the case since it isn't mangling the names.

Xorlev
Thanks for the suggestion, but I've been using .C extension for quite a while and it seems to be fine. It wasn't my choice, however!
vgm64
+1  A: 

Looks like you are compiling and linking individual .C files which are not complete by themselves. You need to compile them first to get .o file and then link the .o files to get the final executable. This all can be done using:

g++ make_pmt.C pmt.C

Also the pmt.C should have only definition of the functions declared in the header file.

codaddict
+6  A: 

pmt.C would look like this:

#include "pmt.h"

using namespace std;

double CPMT::GetGain() {return gain;}
double CPMT::GetVoltage() {return voltage;}

void CPMT::SetGain(double g) {gain=g;}
void CPMT::SetVoltage(double v) {voltage=v;}

I compiled it like this:

g++ make_pmt.C pmt.C

You also need to add a constructor and initialize gain and voltage.

Eddy Pronk
I had read if you leave out a constructor it automatically uses the default one with no arguments supplied. Yeah? And I just didn't initialize the variables because it was just an instructive example. Or is it absolutely to have something more than `double gain, voltage`?
vgm64
It will generate a default constructor, but gain and voltage will not be initialized and contain garbage.
Eddy Pronk
+4  A: 

In pmt.c, you're redefining the class. Instead, you should just define its functions:

double CPMT::GetGain() { return gain; }
double CPMT::GetVoltage() {return voltage;}

void CPMT::SetGain(double g) {gain=g;}
void CPMT::SetVoltage(double v) {voltage=v;}

Also, you need to make sure that you're providing both object files to the linker.

Alex - Aotea Studios
CPMT:: should be before every function name
Draco Ater
Thanks Draco, you're right of course.
Alex - Aotea Studios
+6  A: 

First some taxonomy.
This

class CPMT {
    public:
        double GetGain();
        // ...
};

is defining a class without also defining the member functions. This

class CPMT {
    public:
        double GetGain() {return gain;}
        // ...
};

is defining the same class, with also defining its member functions (implicitly) inline. This

double CPMT::GetGain() {return gain;}
// ...

is defining the member functions (not inline).

Now, if you want to separate implementation from interface, your header needs to define the class, while your implementation file needs to define its member functions. So the pure class definition

class CPMT {
    public:
        double GetGain();
        //...
};

goes into the header file and the implementation

double CPMT::GetGain() {return gain;}
// ...

goes into the implementation file - except for those member functions you want to implement inline. Since inline asks the compiler to substitute a function's implementation for every call to it, the implementation must be present where the function is called. That's why the implementations of inlined functions must be in header files.

There are two ways to inline a member function. One is to define it within its class's definition

class CPMT {
    public:
        double GetGain() {return gain;}
        // ...
};

which implicitly makes it inline. The other is to explicitly inline it

class CPMT {
    public:
        double GetGain();
        //...
};

inline double CPMT::GetGain() {return gain;}
// ...

In both cases the implementation must be in the header file.

sbi
A good link on inline functions is this: http://www.parashift.com/c++-faq-lite/inline-functions.html
Space_C0wb0y
+1: I agree. A lot of modern C++ libraries are header-only nowadays. I try to write as much code as I can in headers and only put implementations if I have to.
Eddy Pronk
@Eddy: I disagree. There's reasons for making things `inline` and there's reasons against doing so. (For starters, lot's of modern C++ libraries are header-only nowadays because most of their stuff is templates, which _have_ to be in headers. Also, the case for inlining code in _libraries_ is much stronger than it is for _application code_.)
sbi
@sbi: There is no good guideline for this, but the point I'm trying to make is that it speeds up development. It saves time not having to keep a header and cpp file in sync. If there is a concrete reason to change it, you can change it which is cheap. see http://stackoverflow.com/questions/2174657/when-are-header-only-libraries-acceptable
Eddy Pronk
@Eddy: Have you ever had to work on an all-template header in a several MLoC project? Something basic, like error handling, or string manipulation, that's been used throughout the whole code base? You change the implementation of some helper function for a helper class that's only used by some other template's private implementation and wait 20mins for the compiler. And then you realize that a few of your valued co-workers ignored your documentation, just looked at the code, and relied on an implementation artifact you were about to change. Inlining can _very_ seriously slow down development.
sbi
Thanks for the amazing answer. You've provided a tutorial for the exact subject I was hoping to get an answer to, and your and @Eddy Pronk's comments were very instructional too. Thank you very much.
vgm64