8

Here's a program, where I am trying to call the class constructor multi::multi(int, int), in the function void multi::multiply(). The output is

30

30

instead of expected

30

25

Why?

#include <iostream.h>

class multi{
    private:
        int a;
        int b;
    public:
        multi(int m, int n){
            a = m;
            b = n;
        }

        void multiply(){
            cout << "\n\n" << a*b;
            multi (5, 5);
            cout << "\n" << a*b;
        }
};

main(){
    multi x(5,6);

    x.multiply();
return 0;
}
Community
  • 1
  • 1
DragonClaw
  • 195
  • 2
  • 2
  • 7
  • The point of a _constructor_ is to _construct_ your object. Unless you use dark magic, only one constructor is called, only one time, for any given object. Once you need to do additional treatment, that's not the job for a constructor anymore. Just do `a = 5; b = 5;` instead your `multiply` method. – zneak Jan 18 '13 at 15:30
  • 1
    Better to use initialization lists in constructors i.e. `multi(int m, int n) : a(m), b(n) {}` – Ed Heal Jan 18 '13 at 15:33
  • 1
    Alternative which works: `*this = multi (5, 5);` – MSalters Jan 18 '13 at 15:35
  • 1
    @MSalters, this might work in certain cases, but in general, completely overwriting the object inside of its own member function is a very bad idea. – Dima Jan 18 '13 at 15:37
  • 1
    @Dima: The assignment operator has a somewhat unusual _syntax_, but that in no way affects safety. Whether you call `*this = X`, `this->operator=(X)` or `this->Foo(X)`, the only thing which affects safety is what the called code actually does. – MSalters Jan 18 '13 at 15:42

5 Answers5

9
multi (5, 5);

It creates a temporary object, and gets destroyed by the end of the full expression. It doesn't do multiplication or printing.

To see the desired output, you can add a reset() member function to your class:

class multi{
    private:
        int a;
        int b;
    public:

        multi(int m, int n) : a(m), b(n) {}   //rewrote it

        void reset(int m, int n) { a = m; b = n; }  //added by me

        void multiply(){
            cout << "\n\n" << a*b;
            reset(5, 5);  //<-------------- note this
            cout << "\n" << a*b;
        }
};

By the way, prefer using member-initialization-list when defining constructors.

Community
  • 1
  • 1
Nawaz
  • 353,942
  • 115
  • 666
  • 851
  • 1
    Okay. Thanks for the edited answer. I get it now. The constructor when called by the member function creates another new temporary object for the class, instead of working on the object it was called from (In this case multi x). – DragonClaw Jan 18 '13 at 15:50
2

When you're calling the constructor multi(5, 5) you're actually creating a temporary object that is immediately destructed.

Steve
  • 1,215
  • 6
  • 11
1

This doesn't work, because multi(5, 5); creates a temporary object of class multi, which is immediately destroyed, because it is not used for anything

Since multiply() is a member function of class multi, it has access to the private members, so it can just set a and b directly. You can get your expected output by rewriting multiply like this:

    void multiply()
    {
        cout << "\n\n" << a*b;
        b = 5;
        cout << "\n" << a*b;
    }
Dima
  • 38,860
  • 14
  • 75
  • 115
1

You can't call constructors like this. What your code does is create a new temporary instance of multi, which gets discarded immediately.

Once an object is constructed, you can't call its constructor again. Create an assign() function or something similar in your class.

Angew is no longer proud of SO
  • 167,307
  • 17
  • 350
  • 455
0

You can't call a constructor of an already created object. What you are doing in code is, creating a temporary object. Compiler will report error if you do try to do this->multi(5,5).