views:

166

answers:

4

That may sound a little confusing. Basically, I have a function

CCard newCard() 
{
    /* Used to store the string variables intermittantly */
    std::stringstream ssPIN, ssBN;
    int picker1, picker2;
    int pin, bankNum;

    /* Choose 5 random variables, store them in stream */
    for( int loop = 0; loop < 5; ++loop )
    {
        picker1 = rand() % 8 + 1;
        picker2 = rand() % 8 + 1;
        ssPIN << picker1;
        ssBN  << picker2;
    }
    /* Convert them */
    ssPIN >> pin;
    ssBN  >> bankNum;

    CCard card( pin, bankNum );
    return card;
}

that creates a new CCard variable and returns it to the caller

CCard card = newCard();

My teacher advised me that doing this is a violation of OOP principles and has to be put in the class. He told me to use this method as a constructor. Which I did:

CCard::CCard()
{
    m_Sperre   = false;
    m_Guthaben = rand() % 1000;

    /* Work */

    /* Convert them */
    ssPIN >> m_Geheimzahl;
    ssBN  >> m_Nummer;
}   

All variables with m_ are member variables. However, the constructor works when I initialize the card normally

CCard card();

at the start of the program. However, I also have a function, that is supposed to create a new card and return it to the user, this function is now broken.

The original command: card = newCard(); isn't available anymore, and card = new CCard(); doesn't work. What other options do I have? I have a feeling using the constructor won't work, and that I probably should just create a class method newCard, but I want to see if it is somehow at all possible to do it the way the teacher wanted.

This is creating a lot of headaches for me. I told the teacher that this is a stupid idea and not everything has to be classed in OOP. He has since told me that Java or C# don't allow code outside of classes, which sounds a little incredible. Not sure that you can do this in C++, especially when templated functions exist, or generic algorithms. Is it true that this would be bad code for OOP in C++ if I didn't force it into a class?

EDIT:

I would like to thank everyone for their helpful answers! However, I believe that my phrasing of the question is a little screwed up, and I think people don't understand what I am looking for.

I do not want to initialize another member of type CCard. I want to intitialize

CCard card once and then give card new values through the constructor, because this is what the teacher told me to do. I do not want to create a new CCard object, just use the same variable with new values over again.

This is why I said it probably won't work with the constructor. So I have a function that is supposed to take the initialized variable card and then call the constructor again( "What?" is what I told the teacher ) and then give it new values.

Example code:

void foo()
{
    /* Initialize card with constructor */
    CCard card;
    /* Give it new values through the constructor AGAIN... */
    card;
}

This is the actual question. I'm sorry if I confused everybody xD

+1  A: 

I'm also new to C++, but I think you might need a copy constructor to return objects from functions:

CCard::CCard (const CCard &rhs)
    : m_Gehiemzahl(rhs.m_Geheimzahl) // and so on for each member variable
{
}

Don't forget to add the prototype to your class declaration.

dreamlax
Copy constructor is automatically provided and works ok if you only have non dynamic members
Nikko
You do, but the compiler will generate a default copy constructor, and since his `CCard` seems to just use primitives it'll be fine (the default just does `this->foo = other->foo;` for each member field)
Michael Mrozek
@all: Cool, thanks, good to know.
dreamlax
+8  A: 

Just some basic pointers, because you have some misunderstandings.

This doesn't intialize a CCard, object. It declares card to be a function returning a CCard and taking no parameters.

CCard card();

If you want to construct a CCard object then just do this.

CCard card;

Your constructor will be called and card should be initialized correctly.

The expression new CCard() dynamically creates a CCard object and "returns" a pointer to a CCard object which you would then need to delete. I don't recommend using this until you've successfully created and understood using local objects first.

EDIT in response to question edit

A constructor can only be called once in an object's lifetime so you can't ever 're-construct' an object. If your class is assignable, though, you can typically assign it the value of a default constructed temporary:

card = CCard();
Charles Bailey
There's a good SO question on [the most vexing parse](http://stackoverflow.com/questions/2318650/is-no-parentheses-on-a-c-constructor-with-no-arguments-a-language-standard)
Michael Mrozek
@Charles Bailey: Thanks for the input, but the compiler is telling me that there is not valid default constructor available for this. And a big thanks for the clear-up on card() and card. That was really a big misunderstanding on my part!
SoulBeaver
@Soul: I suspect you've made it private. (`class` defaults accessibility to private by default.) It should be `class foo { public: foo() {} };`
GMan
@GMan: It's not. When you say default constructor, I am assuming that class CCard { CCard() { /* The work I did up there */ } When you say default constructor, do you really mean just a function CCard () {}? Because that I don't have.
SoulBeaver
@SoulBeaver: I mean the former. In that code you just showed, the default constructor is *private*. You need to label it public first, like in my example. (By the way, use back-ticks (\`) around stuff to format inline code. This: \`blah blah code\` becomes `blah blah code`. I placed a backslash before the tick to escape it from formatting, in the former block.)
GMan
@SoulBeaver: default constructor means a constructor taking no parameter, you can still do anything in the body.
Matthieu M.
@GMan, @Matthieu M.: Thanks for the input, but I think I found the actual problem: I didn't state my question and problem clearly. If you have the time I would really appreciate if you could look at the edit and see what I actually meant was not simply initializing members of the class, but initializing the same member of the class again.
SoulBeaver
@Charles Bailey: Thanks for the update! I don't know what assignable classes are, but I'll go look it up :)
SoulBeaver
+3  A: 
CCard card;

is the way to use the default constructor.

But your teacher is wrong, in C++ we will use free functions to extend functionalities very frenquently.

In your case, it is wise to use the constructor because it is meant this way, except if you have special needs you don't need to do a newCard: this will generate unnecessary copies.


"new" is reserved for pointers, so you have to use something like this:

 CCard * card = new CCard();

And you have to explicitly delete them. But pointers should be used only when necessary, and if possible not 'raw' but smart. (see smart pointers like boost::shared_ptr)


FOR YOUR EDIT

You can give your CCard new values using:

-) a method of CCard like:

card.newValues();

The constructor could also call "newValues()" to avoid being redundant.

-) a free function (or a member of another class) that takes a CCard as a reference parameter:

 void newValues( CCard & card ) { /* set new values */ };

 newValues( card );

-) Directly

 card.set( /*values*/ );

Maybe with a set of accessors (set/get) if it means something for you class.

The question you have to ask yourself is whether you want to change your class internals with accessors, or you only want to change its value inside the class.

-) If you really want to call the constructor again, you can make a copy:

 CCard c;
 c = CCard();

But it's just like creating a new CCard.

Nikko
`std::auto_ptr` / `std::unique_ptr` are perhaps better to begin with than `shared_ptr`.
Matthieu M.
Yes the problem for beginners is to explain std::auto_ptr should not be used inside containers
Nikko
@Nikko: Thanks for answering to my edit ^^ As I thought, it's only cumbersome. Oof, teachers.But thanks for clarifying that!
SoulBeaver
A: 

The point your teacher is trying to make is that you don't need new objects. Instead you need to provide a way to interact with/manipulate objects you currently have. You don't need to reconstruct the object, simply change the object's internal state.

Of course there's a big move to "functional language principles" where mutable state is considered a big source of bugs. But mutable state is not, in itself, bad; it's simply the fact that making a change in an object's internal state is not always obvious. You want to make it obvious.

Max Lybbert