1

I'm trying to implement BigInt for C++ and have run into a problem with copy constructors. You can see I commented out my original code for the copy constructor which was just *this = orig;. But I have found that you need to use pointer instead to do this. I'm not entirely sure how this entirely works however and currently the code doesn't properly make a copy constructor.

-BigIntVector is a custom vector class. Compare to STL Vector.

BigInt.h:

class BigInt {
private:
    BigIntVector bigIntVector;
    bool isPositive;
    int base;
    unsigned int skip;

    BigIntVector* ptr; //pointer to copy?

public:
    // copy constructor
    BigInt(BigInt const& orig);

    // constructor where data value is passed as a long
    BigInt(long num);

    // destructor
    ~BigInt();

    // binary '+' operator
    BigInt operator+(BigInt const& other) const;

    // unary '+' operator
    BigInt operator+() const;

    //more operator unloading functions

Here is my current implementation of the constructors in BigInt.cpp:

// copy constructor
BigInt::BigInt(BigInt const& orig) {
    ptr = new BigIntVector;
    *ptr = *orig.ptr;
    //*this = orig;
}

// constructor where operand is a long
BigInt::BigInt(long num) {
    //this->data = num;

    ptr = new BigIntVector;

    base = 10;

    int sizeOfLong = 0; //holds size of num
    int tempNum = num; 

    //get size of num
    if (tempNum == 0) {
        sizeOfLong = 1;
    }
    while (tempNum != 0)
    {
        tempNum /= 10;
        ++sizeOfLong;
    }

    //resize vector to match size of long
    bigIntVector = BigIntVector(sizeOfLong);

    if (num < 0) {
        isPositive = false;
        num *= -1;
    }
    else {
        isPositive = true;
    }
    long pushedNum;
    //cout << "num: " << num << endl;
    for (int i = sizeOfLong - 1; i >= 0; --i) {
        pushedNum = (long)(num%base);
        bigIntVector.setElementAt(i, pushedNum);
        num /= base;
    }
}

// destructor
BigInt::~BigInt() {
    //delete *this;
}

//code for overloading operators for BigInt below

Code for BigIntVector constructors:

BigIntVector::BigIntVector(long initialSize)
{
    vectorTotalSize = initialSize;
    vectorIncrementSize = initialSize;

    vectorArray = (long *)malloc(initialSize*sizeof(long));
    for (long i = 0; i < initialSize; i++) vectorArray[i] = 0;

    nextValue = 0;
}
sanic
  • 2,065
  • 4
  • 20
  • 33
  • `*this = orig;` is not a correct way to implement a copy-constructor anyway. The assignment operator should leverage the copy-constructor, not the other way around. – M.M Nov 06 '15 at 02:45
  • updated: "-BigIntVector is a custom vector class. Compare to STL Vector." – sanic Nov 06 '15 at 02:49
  • What is `BigIntVector* ptr;` for in the class definition? Your constructor makes it point to a `new` vector but then does not put anything in this new vector. – M.M Nov 06 '15 at 02:49
  • Apart from special circumstances, do not `new` data structure objects themselves, such as `ptr = new BigIntVector`. Instead make them full members. You may find that the default-generated copy constructor and assignment operator then work automatically for you. – Neil Kirk Nov 06 '15 at 02:55
  • 1
    I think this is a side effect of being a big time Java user. What is the proper syntax for "full members" – sanic Nov 06 '15 at 03:06
  • Use an *existing* bigint libary, such as [GMPlib](http://gmplib.org/) – Basile Starynkevitch Nov 06 '15 at 06:03

2 Answers2

1

Well. I don't know what BigIntVector* ptr; //pointer to copy? is for so I am assuming it is a mistake. Your code should look like this:

class BigInt
{
private:
    BigIntVector bigIntVector;
    bool isPositive;
    int base;
    unsigned int skip;

public:
    // constructor where data value is passed as a long
    BigInt(long num = 0);

    BigInt &operator+=(BigInt const& other);

    // Other accessors...
};

inline BigInt operator+(BigInt a, BigInt const &b) { a += b; return a; }

The main principle here is the Rule of Zero. You design your class so that the default-generated function does the correct thing for: copy-constructor, move-constructor, move-assignment, copy-assignment, destructor.

Then your class definition remains simple. To achieve this goal, all data members must implement those functions correctly -- either by following the Rule of Zero themself, or actually having the functions.

The built-in types all follow the rule, and I am assuming that BigIntVector follows rule of 0,3, or 5. (If not, fix it so that it does!)

Other changes I made are:

  • It is good style for non-mutating operators to be non-member functions. If you find yourself able to put const on a member operator it's a good sign that it should be a non-member. For more on this topic see this thread.
  • Comments should say something additional to what the code says. Putting a comment "destructor" over the destructor, or "binary + operator" over the binary + operator just clutters your screen for no reason.

In your long constructor, int tempNum = num; should be long tempNum = num;; and you should initialize skip.

Community
  • 1
  • 1
M.M
  • 138,810
  • 21
  • 208
  • 365
1

In the real world (outside of homework), a BigInt class should not need an explicit copy constructor. The memory allocation should be delegated to a separate class -- most likely std::vector. If you need a pointer as a class member (unlikely in this case), managing that with std::shared_ptr eliminates the need for a copy constructor.

Here's an earlier post of mine that address your misconceptions about the use of new in C++. The references to C# in that description apply equally well to Java: Is garbage collection automatic in standard C++?

Regarding a question in the comments: "what is the syntax for full members". The comment suggesting that was just saying don't declare the element as a pointer (just leave out the *). To summarize, for this class:

  • You don't need a pointer
  • You don't need a copy constructor
  • You don't need to use the new keyword

In this case, the problem lies in your BigIntVector class. Since that is the one that allocates memory and manages a pointer, that is the one that would require a copy constructor. If this is a homework assignment, I suggest applying the rule of three to your implementation of BigIntVector. You will need all of the following:

  • a copy constructor, which copies the memory
  • an assignment operator, which also copies the memory
  • a destructor, which releases the memory

If it isn't homework, I suggest replacing BigIntVector with std::vector.


It is a bad idea to declare a copy constructor for a non-trivial class because you would generally need to copy every member explicitly (or they will be default-initialized rather than copied) -- better to let the compiler do it for you to avoid mistakes.

If you must have one for this assignment, the correct form would be something like this:

BigInt::BigInt(BigInt const& orig)
 : bigIntVector(orig.bigIntVector)
 , isPositive(orig.isPositive)
 , base(orig.base)
 , skip(orig.skip)
{
// empty body
}

The canonical form of forwarding arithmetic operators to compound assignment operators is some variation of:

T& T::operator += ( T const & b )
{
  ...class-specific math logic that modifies the object...
  return (*this);
}

T operator + ( T const & a, T const & b )
{
  T temp(a); // create a temporary copy of 'a' rather than modifying it
  return temp += b;
}
Community
  • 1
  • 1
Brent Bradburn
  • 51,587
  • 17
  • 154
  • 173
  • Whenever I reuse new instances of the BigInt class it tries to reuse the the last instance. For example, num1 = 10 and num2 = 5 and num3 = num1 + num2 and num4 = ++num1. Num4 would be much higher because it is being added to num3 for some reason. Perhaps it has something to do with the copy constructor and that's why it is necessary in this case. – sanic Nov 06 '15 at 03:40
  • 1
    Who wrote the `BigIntVector` class? It has its own copy semantics -- which you need to understand. A `std::vector` would *duplicate* its contents when you copy or assign it, but maybe `BigIntVector` shares its contents (more like the standard approach in Java). If that is the case, then you probably *do* need a pointer to make your class behave the way that you want, but preferably you would use a [smart pointer](https://en.wikipedia.org/wiki/Smart_pointer) such as `std::shared_ptr` unless that isn't allowed by the assignment. If you use a smart pointer, you don't need a copy constructor. – Brent Bradburn Nov 06 '15 at 03:47
  • I wrote most of it. I updated the post with my constructor for BigIntVector. I believe when copied this would doing the shared contents. I need a copy constructor but I need help figuring out how to set up the pointer. – sanic Nov 06 '15 at 03:51
  • I appreciate the suggestions for BigIntVector. I'm also required to have the copy constructor in BigInt, do you have a suggested implementation for that? – sanic Nov 06 '15 at 04:26
  • You have been super helpful! Thank you! I have one more question. If I wanted to access the copied version of BigInt say in the overloaded += operator function, how can I go about doing that. For instance, say num3 = num1 + num2 so I don't want to have the actual instance of num1 changed to a new number in += (since + redirects to +=), I want the copy of num1 changed to a new number. I hope this makes sense. Thanks again! – sanic Nov 06 '15 at 04:57
  • "`std::shared_ptr` eliminates the need for a copy constructor." -- Not sure what I was thinking about here. Normally, if you need to use a `shared_ptr` as a class member, then you probably would want to disable the copy constructor. A value-type class like BigInt, should use a self-copying data-store like `std::vector`. I guess allowing a `shared_ptr` to be implicitly copied would be fine if the class used a copy-on-write strategy for the backing store. – Brent Bradburn Jun 08 '16 at 14:14