1

Given the following code:

template <class T>
class A {
     T* arr;
     int size;
public:
A(int size) : arr(new T[size]) , size(size) {
}
//..

A& operator=(const A& a){
     if(this == &a){
          return *this;
     }
     this->size = a.size;
     T* ar=new T[a.size];
     for(int i=0 ; i<size ; i++){
          ar[i]=a.arr[i]; // I need to do it at "try-catch" ?
     }
     delete[] this->arr;
     this->arr=ar;
     return *this;
}
     //...
};

When I copy the elements from the given array, do I need to do it with a try-catch clause or not? is it a good idea or not?

Mohammad Kanan
  • 4,452
  • 10
  • 23
  • 47
Software_t
  • 576
  • 1
  • 4
  • 13
  • Aren't there some guarantees about "assignment [should] never throw(s)" in [a valid program in] C++? – user2864740 Jun 30 '18 at 20:18
  • @user2864740 I don't understand your question, but , for example, "std::bad_alloc" can be thrown, no? – Software_t Jun 30 '18 at 20:19
  • 2
    @user2864740 No. Some types might provide such a guarantee, some types might not. – Barry Jun 30 '18 at 20:20
  • See [What is the copy and swap idiom?](https://stackoverflow.com/questions/3279543/what-is-the-copy-and-swap-idiom) for an in-depth explanation of how to do this, with an example that is basically your type. – Barry Jun 30 '18 at 20:21
  • @Barry What you mean while you say "Some types might provide such a guarantee" , if thrown exception "std::bad_alloc" or not it's depends on the heap (and not on the class, no?) – Software_t Jun 30 '18 at 20:24
  • You should be more worried about the memory leak. – juanchopanza Jun 30 '18 at 20:24
  • @juanchopanza It's was exactly my qeustion, if I need to write the line that written in the "for-while" inside "try-catch" so that if thrown "std::bad_alloc" so I will do "delete[] ar" and throw this exception.. – Software_t Jun 30 '18 at 20:27
  • @Software_t - You are copying elements of type `T`. Any restrictions on what `T` can be? Otherwise its operations can very well throw exceptions. – Bo Persson Jun 30 '18 at 20:29
  • Let me guess.... some assignment where you cannot use `std::vector`. Assigning to an existing vector should not throw an exception. Don't use exceptions for normal situations. – JHBonarius Jun 30 '18 at 20:29
  • @BoPersson I don't understand your question.. `T` can be anything, it's generic. – Software_t Jun 30 '18 at 20:31
  • 1
    @Software_t - So in `arr[i]=a.arr[i];` you use `T::operator=(const T&)`. Could that throw? Well - Yes. – Bo Persson Jun 30 '18 at 20:34
  • @BoPersson So according to your answer , I need to write it inside "try-catch" (or at least, it's will be more correct to do it like that). I am right? – Software_t Jun 30 '18 at 20:47
  • 1
    @Software_t - Yes. Or add restrictions on `T`. If it is an `int`, for example, there will be no problem. That was what my first comment was supposed to be about. :-) – Bo Persson Jun 30 '18 at 20:53
  • Software_t: Before you do `this->arr=ar;` why are you not doing `delete [] this->arr;` ?? That is a memory leak. – SJHowe Jun 30 '18 at 21:07
  • @SJHowe Ohh.. Of course, I am sorry, you right. Thank you :) I edited it! – Software_t Jun 30 '18 at 21:10
  • 1
    I think you should not modify the size field of A so early. If the array alloc or T copy does exception, then the A object will be invalid, as its size does not match its allocation. – Gem Taylor Jul 02 '18 at 10:52

1 Answers1

0

I can see that potentially your T copy could throw due to its own alloc failure or other reasons.
On the other hand your A copy could already throw because it had alloc failure.

Currently you would need to handle the destruction because you have not concreted the array that you have allocated, and all the T instances that you have created need to be destroyed if one of them exceptions, perhaps due to allocation failure.

One quick way to fix that would be to hold the array in a unique_ptr. Then it will be destroyed on exiting context.

Another way may be to reconsider your contract on A after the assignment has exceptioned: It must be valid, i.e. survive being used, but perhaps it need not guarantee to still contain its previous contents, nor all the new contents, so you could decide to destroy its existing array before allocating and assigning a new array, then copying the members. You could decide not to reallocate if the size has not changed, but just re-assign - this would leave a mess of new and old members after an exception, but they would all be valid and safe to delete.

Please ensure that size matches the actual attached array size at all times! Your existing code makes this mistake, but in particular that it is set to null and 0 after the delete and before the assignment; and it is only set to new new size after the assignment of the new pointer.

Gem Taylor
  • 5,381
  • 1
  • 9
  • 27