1

I used to use initialization list through Constructor and everything went well. But now I need some exception handling in my class.

So here is some sample code:

1- Without exception handling

class CVector{
    public:
        CVector(const int);
    protected:
        int* pInt;
        int size;
};

CVector::CVector(const int sz) :
    pInt{ new int[sz]}, size{ sz}{

}

The constructor above doesn't check whether an invalid sized is passed, or new failed...

Now I edited the constructor to handle exceptions:

2- With exception handling:

CVector::CVector(const int sz){
    size = sz;
    if(size <=0 )
        throw; // some exception object

    pInt = new int[sz];
    if(nullptr == pInt)
        throw; // some memory exception
}
  • The problem now: Are they exclusive? - I mean How to mix exception handling with initialization-list through Constructor?
WonFeiHong
  • 405
  • 4
  • 15
  • `new` can't return `nullptr` unless you use specify nothrow. It throws [`std::bad_alloc`](http://en.cppreference.com/w/cpp/memory/new/bad_alloc) on failure to allocate. – François Andrieux Jan 03 '18 at 18:56
  • 3
    I don't think this question is really a duplicate of the indicated dupe. It's really about safe resource management when not using a smart pointer. (hint: use a smart pointer). – Richard Hodges Jan 03 '18 at 18:59
  • @RichardHodges Thank you for the hint. And please add an answer. – WonFeiHong Jan 03 '18 at 19:00
  • @RichardHodges: I don't really know why it is closed so quickly? – WonFeiHong Jan 03 '18 at 19:03
  • @WonFeiHong The question was closed because a similar question has already been asked and answered. But that decision has since been reversed. The question is currently open. – François Andrieux Jan 03 '18 at 19:04
  • @FrançoisAndrieux: Ok. Thank you. – WonFeiHong Jan 03 '18 at 19:04
  • 1
    Normally, you would use `std::vector` and avoid all of these problems. However, since you seem to be trying to implement your own vector (perhaps as a learning exercise?) then you could use [`std::unique_ptr`](https://stackoverflow.com/questions/16711697/is-there-any-use-for-unique-ptr-with-array) instead. In any case, you should rely on [RAII](https://stackoverflow.com/questions/2321511/what-is-meant-by-resource-acquisition-is-initialization-raii) rather than trying to catch exceptions and cleaning up manually. A class should only do that manually if that it's specific and unique job. – François Andrieux Jan 03 '18 at 19:05

2 Answers2

2

On first view, it's not clear whether you intend CVector to responsibility for owning the dynamic of integers or not. You probably do.

Almost without fail, you will want each class in your program to manage at most one dynamic resource (such as allocated memory). This is because the job becomes onerous when there is more than one resource.

In your case, the resource is the int array. So let's give CVector the responsibility, and let's manage that responsibility using a unique_ptr:

#include <memory>
#include <cstddef>
#include <algorithm>

struct CVector
{
    CVector(std::size_t initial_size)
    : data_ { std::make_unique<int[]>(initial_size) }
    , size_ { initial_size }
    {
    }

    // we will probably want the vector to be copyable
    CVector(CVector const& other)
    : data_ { std::make_unique<int[]>(other.size_) }
    , size_ { other.size_ }
    {
        auto first = other.data_.get();
        auto last = first + other.size_;
        std::copy(first, last, data_.get());
    }

    // move consruction is defaultable because it is enabled for
    // unique_ptr

    CVector(CVector&&) = default;

    // assignment

    CVector& operator=(CVector const& other)
    {
        auto temp = other;
        swap(temp);
        return *this;
    }

    CVector& operator=(CVector&& other) = default;

    // no need for a destructor. unique_ptr takes care of it

    void swap(CVector& other) noexcept
    {
        using std::swap;
        swap(data_, other.data_);
        swap(size_, other.size_);
    }

private:
    // defer memory ownership to a class built for the job.
    std::unique_ptr<int[]> data_;
    std::size_t size_;
};
Richard Hodges
  • 64,204
  • 6
  • 75
  • 124
1

1) extract size validation check into a separate static method (most likely it will be reused anyway); 2) there is no need to check the value returned by new anymore, it should thrown an std::bad_alloc exception if fails. So your code can become something like this:

class CVector{
public:
    CVector(const int);
    static int Validate_Size(int const size);
protected:
    int * m_pInt;
    int m_size;
};

int CVector::Validate_Size(int const size)
{
   if(size <= 0)
   {
      throw std::invalid_argument{};
   }
   return(size);
}

CVector::CVector(const int size)
:  m_pInt{new int[Validate_Size(size)]}
,  m_size{size}
{}
user7860670
  • 32,142
  • 4
  • 44
  • 75