0

I am trying refactor a piece of C++ code into a class. Right now the code looks like this

USB Usb;
ACMAsyncOper AsyncOper;
ACM Acm(&Usb, &AsyncOper);

I want to move this code into the constructor of a class. Also I want to have the variables Usb, AsyncOper and Acm as member variables of the class.

I wrote it as the following

// eZ430.h

class eZ430
{
    public:
        eZ430();    
    private:
        USB Usb;
        ACMAsyncOper AsyncOper;
        ACM Acm;
};


// eZ430.cpp
#include "eZ430.h"

eZ430::eZ430() {
    USB Usb;
    ACMAsyncOper AsyncOper;
    ACM Acm(&Usb, &AsyncOper);
}

But this doesn't seem to work. I am very new to C++ and can't get it to work.

Kindly let me know how to implement it. Thanks.

Edit: When I have the following code in the constructor

eZ430::eZ430() {
    USB Usb;
    ACMAsyncOper AsyncOper;
    ACM Acm(&Usb, &AsyncOper);
}

I get the error error: expected identifier before '&' token

And when I change it to

eZ430::eZ430() {
    USB Usb;
    ACMAsyncOper AsyncOper;
    ACM Acm(&Usb, &AsyncOper);
}

I get the error no matching function for call to 'ACM::ACM()'

Sudar
  • 18,954
  • 30
  • 85
  • 131
  • 1
    Are you getting any errors? Tell us what they are. – David G Jun 01 '13 at 16:51
  • What you want to do -- have objects as member variables of another class -- is *in general* possible, but there may be things about the classes involved that make it not work. Also, you definitely shouldn't be redeclaring all your data members as local variables inside the constructor. – zwol Jun 01 '13 at 16:54
  • @0x499602D2 Updated the question with the error that I am getting. – Sudar Jun 01 '13 at 16:55

1 Answers1

5

Your constructor should initialize Acm through its member-initializer list:

eZ430() : Acm(&Usb, &AsyncOper)
{}

We do this because ACM doesn't have a default constructor, and we have to make sure that default-construction of eZ430 causes the specialized-construction of Acm.

And leave the body empty as there is no reason to recreate the Usb and AsyncOper data-members inside the constructor. Besides, doing ACM Acm(&Usb, &AsyncOper) potentially causes Undefined Behavior because you're accessing the addresses of local variables that will go out of scope when the constructor body closes. If you use those addresses elsewhere that will cause the Undefined Behavior.

Related: C++ Member Initialization List

Community
  • 1
  • 1
David G
  • 94,763
  • 41
  • 167
  • 253
  • To elaborate further, I'm guessint Acm doesn't have a default constructor. That's why the member-initializer is necessary. – Lance Jun 01 '13 at 16:55
  • Not only is there no reason to recreate the data members inside the constructor, it's actively wrong: the data members wind up being default-initialized, or worse, initialized with pointers to locals which immediately go out of scope, thus arming an undefined-behavior bomb. – zwol Jun 01 '13 at 16:58