0

Suppose I have a class A, which uses dynamic memory allocation, like an array or a matrix. During the creation of an object A, through the constructor are passed the parameters which determine how much memory space is being allocated.

class A
{
    int * ptr;
    int size;
    void allocate() noexcept;
    void destroy() noexcept;
   public:
    A(int) noexcept;
    ~A() noexept;
}
void A::destroy() noexcept
{
  if(ptr!=nullptr)
     delete [] ptr;
}
A::~A() noexcept
{ destroy();}
void A::allocate() try
{
   ptr = new int[n];
}
catch (std::bad_alloc & ex)
{
   std::cerr << ex.what();
   destroy();
}
A::A(int n) noexcept : size(n) { allocate(); };

Are these kinds of things a good practice, design wise? What would happen to an object that cannot be created in the intended way? Would it remain "alive", like a zombie object, or would it be destroyed?

What to do in the matrix scenario, where the matrix is allocated with the multiple new statements?

Smart pointers, STL and that kind of stuff is not an option here.

READ ME: This class is a demonstrative class, purely made for this kind of issue. It doesn't follow the rule of 0/3/5, because it would be too much code to write just for a question, and it isn't important for this question. There already exists a bunch of questions regarding those issues.

Shocky2
  • 504
  • 6
  • 24
  • 1
    Why not let the things that should throw, throw? – juanchopanza Aug 29 '17 at 21:36
  • 1
    Why call `destroy` on failed allocation? Failure to allocate means you don't have anything to `delete`. – François Andrieux Aug 29 '17 at 21:37
  • 1
    "Is this a good solution?" - to what problem? –  Aug 29 '17 at 21:37
  • 2
    If your constructor can't succeed, do not force it to appear to succeed. Your problem vanishes if you don't insist on catching thrown exceptions. Is there a specific reason why you want the constructor to be `noexcept`? – François Andrieux Aug 29 '17 at 21:37
  • @NeilButterworth Design wise. Is it a good solution design wise, for a class that uses dynamic memory allocation. Is it not obvious? – Shocky2 Aug 29 '17 at 21:43
  • Designing what? –  Aug 29 '17 at 21:44
  • @FrançoisAndrieux So, I should let it throw, and there would be no problems? What would happen if it throws? Would it destroy the object that is "half-created"? – Shocky2 Aug 29 '17 at 21:46
  • 1
    @Shocky2 A constructor that throws indicates you should treat the object as though it never existed. Stack unwinding occurs in the constructor's body from the point where the exception was thrown. If the exception originated from the initialization list, members that have been initialized are destroyed in reverse order. You should read the answers here : https://stackoverflow.com/questions/32323406/what-happens-if-a-constructor-throws-an-exception Be sure to write your constructors in such a way that they are exception safe. – François Andrieux Aug 29 '17 at 21:51
  • @FrançoisAndrieux Thank you, Sir. You are a gentleman and a scholar! So, if the class had a matrix member, and not all of the matrix could be allocated, the destructor would not be called, but I would still have a few rows of the matrix allocated, but not the whole matrix. Where is it best to handle that half-allocated matrix? – Shocky2 Aug 29 '17 at 21:55
  • @Shocky2 If you are allocating your matrix with a single `new` than it's not possible to have a partial matrix. If you are using multiple `new` statements, than you will have to manually `delete` each allocation that already succeeded. The same is true for any other leakable resource you may have acquired. That means catching the exception, cleaning up and then rethrowing it at the end. This is a very poor solution, use RAII wrappers instead. If I were you, and I couldn't use `std::` (why?) I would write my own simplified `unique_ptr` to avoid having to deal with these issues. – François Andrieux Aug 29 '17 at 21:58
  • @Shocky2 Additionally, you will encounter problems when using your `class` since you did not implement or delete the copy constructor and copy assignment operator. Read about [the rule of 3/5/0](http://en.cppreference.com/w/cpp/language/rule_of_three). Again, you don't have this problem if you use proper RAII types. Copying your class will cause the pointers to be copied, leading to multiple instances referring to the same internal data and, eventually, deleting it multiple times. – François Andrieux Aug 29 '17 at 21:59
  • @FrançoisAndrieux The class was purely for demonstrative purposes of this issue, I am familiar with the said rule. Thank you a lot! I am using multiple new statements in the matrix, so I do cleaning-up in the catch block, and then re-throw it. Thanks! You answered my question fully. If you want to, you can write it down in the form of an answer, I will gladly accept it. Many thanks. You are an awesome human being. – Shocky2 Aug 29 '17 at 22:08

2 Answers2

1

Q. Are these kinds of things a good practice, design wise?

A. There is no harm in doing this per-se and it depends on the requirements. In general, this is good ONLY if you want to ensure that the constructor never throws (which probably is your intention here) and is MEANINGFUL if the data you are allocating for is not critical to the operation of the object. In most cases its always best to throw an exception in case there is a failure in the constructor. Note that if you use this, you would also need to check for NULL ever time you access ptr[].

Q. What would happen to an object that cannot be created in the intended way? Would it remain "alive", like a zombie object, or would it be destroyed?

A. This can happen in 2 ways : 1. Failure to allocate memory for object A itself. In this case it would depend on the method used to allocate memory. If its "new" then it would throw an exception by default. 2. Failure to allocate memory for the integer array in allocate. In this case you would catch the exception. In the catch block, deallocation of ptr using destroy() is not necessary in this case as the only way in which the try block would fail is memory cannot be allocated for the array of n integers. A's constructor would hide this failure and the memory allocated for object A would remain as-is.

Q. What to do in the matrix scenario, where the matrix is allocated with the multiple new statements?

A. Do you mean what would happen if you allocate memory for ptr in a for loop/multiple new statements? If so, the behavior would be same, except in this case any memory already allocated to ptr[] would remain as-is and you would need the destroy() call in catch block to de-allocate any memory allocated to ptr[]. Note in destroy you would again need to separately check for NULL and deallocate for each array element.

Madhu
  • 29
  • 3
0

No, these things are a bad idea.

The root problem is composability. Like every OO language, C++ allows user-defined types. These types can contain members of different types. And C++ defines rules how such composed types are created and destroyed.

The C++ rules are written with exceptions in mind. They are not written with zombie objects in mind. Exceptions when creating a member are propagated, and its siblings are properly destroyed. Zombie-status of members is not propagated, and siblings are hard to clean up (if at all).

Now you might think that instead of relying on the C++ rules you can introduce a company-wise coding standard that composite objects should have an bool isZombie() const method that checks all members. But C++ has template containers. std::vector<YourClass> does not have an isZombie() method; std::vector<> does not follow your company's coding standard. However, std::vector<T> does propagate exceptions from T::T(). And when it does, it won't corrupt internal state. Not even halfway through a vector.resize().

MSalters
  • 173,980
  • 10
  • 155
  • 350