Constructor throws wrong exception
CodeGuru Home VC++ / MFC / C++ .NET / C# Visual Basic VB Forums Developer.com
Page 1 of 2 12 LastLast
Results 1 to 15 of 25

Thread: Constructor throws wrong exception

  1. #1
    Join Date
    Dec 2013
    Posts
    75

    Constructor throws wrong exception

    Hey guys!

    Writing a smart array class for my C++ class. I'm running into a problem where the constructor apparently throws the wrong exception. Compiled on G++ with C++11 extensions enabled.

    Code:
    // headers
    #include <iostream>
    #include <utility>
    #include <cctype>
    
    // stuff we need from namespace std
    using std::cout;
    using std::cin;
    using std::endl;
    using std::move;
    using std::isalpha;
    
    //exception class
    class InvalidSize {};
    class InvalidInput {};
    // Array class
    class Array
    {
    	public:
    	//constructors
    	Array()=delete; // Array must have a size, and cannot be stored in a structure.
    	Array(int size); //Throws InvalidSize and InvalidInput
    	//copy constructor
    	Array(const Array& rhs);
    	//move constructor
    	Array(Array&& rhs);
    	//conversion from regular array
    	Array(int* array, int size);
    	// copy assignment operator
    	Array& operator=(const Array& rhs);
    	//move assignment operator
    	Array& operator=(Array&& rhs);
    	//destructor
    	~Array();
    	//index operator
    	int& operator[](int index) const; 
    	//conversion to regular array
    	operator int*() const;
    	//get length of array
    	int getsize() const;
    	//data fields
    	private:
    	int size;
    	int* array;
    };
    
    //main
    int main()
    {
    	// size of array
    	int size;
    	//ask for array size
        cout << "How big do you want your array to be?" << endl;
        cin >> size;
        try
        {
        	Array array(size);  
        // get data
        cout << "Enter the numbers you'd like to store: " << endl;
        for (int i = 0; i < array.getsize(); i++)
        {
        	int val;
        	cin >> val;
        	array[i]=val;
        }
        //display array contents
        cout << "Displaying array contents." << endl;
        for (int i=0; i < array.getsize(); i++)
        cout << array[i] << endl;
        }
        //handle exceptions
          catch (InvalidSize)
        {
        	cout << "Arrays must have at least two objects!" << endl;
        }
        catch (InvalidInput)
        {
        	cout << "Integers only!" << endl;
        }
     //done
     return 0;
    }
    
    //regular constructor
        
      Array::Array(int s): size(s)
      {
      	// validate input
      	if (isalpha(s))
      	throw InvalidInput();
      	else if (s < 2)
      	throw InvalidSize();
      	//otherwise build the array
      	else
      	{
      		cout << "Building array." << endl;
      		array = new int[size];
      	}
      }
      
    //copy constructor
    Array::Array(const Array& rhs):size(rhs.size), array(new int[size])
    {
    	//delegate to copy assignment operator
    	*this = rhs;
    } 
    
    //move constructor
    Array::Array(Array&& rhs):array(0)
    {
    	//delegate to move assignment operator
    	*this = move(rhs);
    }
    
    //array conversion constructor
    Array::Array(int* carray, int s):Array(size)
     {
     	for (int i = 0; i < s; i++)
     	array[i] = carray[i];
     }
     
     //copy assignment operator
     Array& Array::operator=(const Array& rhs)   
     {
     	if (this==&rhs)
     	cout << "Self-assignment prohibited!" << endl;
     	if (size != rhs.size)
     	cout << "Arrays must be equal in size!" << endl;
     	else
     	for (int i = 0; i < size; i++)
     		array[i] =rhs.array[i];
     		return *this;
     }
     
     //move assignment operator
     Array& Array::operator=(Array&& rhs)
     {
     	//not checking for self-assignment, as rhs is likely a temporary
     	if (size != rhs.size)
     	cout << "Arrays must be same size!" << endl;
     	else
     	{
     		cout << "Moving array." << endl;
     		for (int i = 0; i < size; i++)
     			array[i] = rhs.array[i];
     			delete[] rhs.array;
     			rhs.array = 0;
     			return *this;
     	}
     }
     
     //destructor
    Array::~Array()
    {
    	//deallocate the wrapped array
    	delete[] array;
    	array = 0;
    }
    
    //index operator
    int& Array::operator[](int index) const
    {
    	//perform bounds checking
    	if (index < 0 || index >= size)
    	cout << "Array index must not be negative or greater then or equal to array size." << endl;
    	else
    	return array[index];
    }
    
    //conversion to regular array
    Array::operator int*() const
    {
    	return array;
    }
    
    //getter for size
    int Array::getsize() const
    {
    	return size;
    }
    When I try to check the error-handling code, when I enter a size less then two, Array's ctor throws InvalidSize, and gets caught, all good. But when I enter a letter, the ctor also seems to throw InvalidSize!

    Weird. Any advice? Or general comments?

    Thanks!

  2. #2
    Join Date
    Dec 2012
    Location
    England
    Posts
    2,842

    Re: Constructor throws wrong exception

    Code:
    int size;
    ...
    cin >> size;
    As size is defined to be of type int, if you try to enter a non integer value, the cin stream fails and size is not set. You're not checking the input stream for a problem after extracting data. The same applies to when you extract from the input stream the numbers to store. You are not checking the state of the stream to see if an error has occured.

    See http://www.cplusplus.com/reference/istream/istream/
    All advice is offered in good faith only. You are ultimately responsible for effects of your programs and the integrity of the machines they run on.

  3. #3
    Join Date
    Dec 2013
    Posts
    75

    Re: Constructor throws wrong exception

    Ah. I fixed it by
    Code:
    if (cin.fail())
    throw InvalidInput();
    else
    //useful work
    every time I used cin. Now, how do I avoid my code becoming a massive nested if-then-else structure of tests and throws?

  4. #4
    Join Date
    Apr 1999
    Posts
    27,446

    Re: Constructor throws wrong exception

    Quote Originally Posted by TSLexi View Post
    I'm running into a problem where the constructor apparently throws the wrong exception. Compiled on G++ with C++11 extensions enabled.
    1) Did you run the program under the debugger to see why it chooses this exception?

    Other issues:
    1) Your Array::Array(int) constructor is flawed -- what if I tried to create an array of 65 integers? Since 65 is 'a', the isalpha() would have returned 1, causing an exception to be thrown.

    2) Your assignment operator has serious side effects -- if you assign an array of a different size, the assignment doesn't fulfill the request. The problem with side-effects is that the compiler can call the assignment operator "behind your back", thus giving you bogus copies floating around that you didn't suspect. The assignment operator (and copy constructor) fulfill one purpose, and that is to assign and copy. Not doing so can raise serious run-time bugs that are difficult to diagnose. When a programmer does this:
    Code:
    T x;
    T y;
    //...
    y = x;
    The y should be equivalent and behave exactly the same as x. In other words, after that assignment, if I used y instead of x throughout the program, the program needs to behave exactly the same as if I used x instead of y.

    Also, your copy constructor works contextually differently than the assignment -- this leads to confusion when using the class. For example, if I can do this:
    Code:
    T x;
    //... assume you added items to x
    T y = x;
    Then I should be able to do this:
    Code:
    T x;
    T y;
    // assume you added items to x
    y = x;
    The first calls the copy constructor, the second calls the assignment operator. Why should the program behavior be different if I use one over the other?

    Regards,

    Paul McKenzie
    Last edited by Paul McKenzie; January 24th, 2014 at 06:25 AM.

  5. #5
    Join Date
    Dec 2013
    Posts
    75

    Re: Constructor throws wrong exception

    I rewrote the assignment operators to throw an Out Of Bounds exception when the lhs and rhs do not have the same sizes.

  6. #6
    Join Date
    Apr 1999
    Posts
    27,446

    Re: Constructor throws wrong exception

    Quote Originally Posted by TSLexi View Post
    I rewrote the assignment operators to throw an Out Of Bounds exception when the lhs and rhs do not have the same sizes.
    Which is the wrong thing to do. You don't introduce side-effects in an operation such as assignment, which again, you have no complete control over (unless you turn off assignment altogether).

    I also edited my previous post to give another example of why you shouldn't do this.

    Regards,

    Paul McKenzie

  7. #7
    Join Date
    Dec 2013
    Posts
    75

    Re: Constructor throws wrong exception

    So, how should I handle copying between arrays of different sizes?

  8. #8
    Join Date
    Apr 1999
    Posts
    27,446

    Re: Constructor throws wrong exception

    Quote Originally Posted by TSLexi View Post
    So, how should I handle copying between arrays of different sizes?
    In the way you implemented your class, you have to

    1) Create new memory with the new size
    2) Copy the passed in data to the new memory
    3) Delete the old memory

    Otherwise, you should have written your copy constructor to do this work, and not delegate it to the assignment operator. Instead, you delegate the assignment operator to call the copy constructor. Then you can do the copy and swap idiom to do the assignment:
    Code:
    Arrray& operator=(const Array& rhs)
    {
        Array tmp(rhs);
        swap(*this, tmp);
        return *this;
    }
    Now you don't even need to test for self-assignment.

    http://stackoverflow.com/questions/3...and-swap-idiom

    Regards,

    Paul McKenzie

  9. #9
    Join Date
    Dec 2013
    Posts
    75

    Re: Constructor throws wrong exception

    Rewrote the assignment operators to work properly
    Code:
    //copy assignment operator
     Array& Array::operator=(const Array& rhs)   
     {
     	if (this==&rhs)
     	throw;
     	if (size != rhs.size)
     	{
     		size=rhs.size;
     		delete[] array;
     		array = new int [size];
     	}
     	for (int i = 0; i < size; i++)
     		array[i] =rhs.array[i];
     		return *this;
     }
     
     Array& Array::operator=(Array&& rhs)
     {
     	//not checking for self-assignment as rhs is most likely temporary
     	cout << "Moving array." << endl;
     	
     	// reallocate array size if need be
     	if (size != rhs.size)
     	{
     		size=rhs.size;
     	    delete[] array;
     	    array = new int[size];
     	}
     	for (int i = 0; i < size; i++)
     	array[i] = rhs.array[i];
     	delete[] rhs.array;
     	rhs.array=0;
     	return *this;
     }

  10. #10
    Join Date
    Apr 2000
    Location
    Belgium (Europe)
    Posts
    4,100

    Re: Constructor throws wrong exception

    Quote Originally Posted by TSLexi View Post
    Ah. I fixed it by
    Code:
    if (cin.fail())
    throw InvalidInput();
    else
    //useful work
    every time I used cin. Now, how do I avoid my code becoming a massive nested if-then-else structure of tests and throws?
    by not writing unnecessary else clauses... for starters...

    in the above, the else is not needed. If you enter the if, you get thrown out of the function, so the "else" doesn't really serve any purpose. Doing this rigourously will make your code more readable because the important code that does the real work isn't as deeply indented.

  11. #11
    Join Date
    Apr 2000
    Location
    Belgium (Europe)
    Posts
    4,100

    Re: Constructor throws wrong exception

    Another step (with some setup) would be to write thrower-functions.

    Code:
    void ThrowInvalidInput(bool bCondition)
    {
       if (bCondition)
            throw InvalidInput();
    }
    
    
    void foo()
    {
         // early outs...
         ThrowInvalidInput( cin.fail() );
    
         // Usefull work
         DrinkBeer();
         EatBagOfCrisps();
         WatchTV();
         ScratchBalls();
    }

  12. #12
    Join Date
    Dec 2013
    Posts
    75

    Re: Constructor throws wrong exception

    Okay, here's the final version
    Code:
     // headers
    #include <iostream>
    #include <utility>
    #include <cctype>
    
    // stuff we need from namespace std
    using std::cout;
    using std::cin;
    using std::endl;
    using std::move;
    using std::isalpha;
    
    //exception class
    class InvalidSize {};
    class InvalidInput {};
    // Array class
    class Array
    {
    	public:
    	//constructors
    	Array()=delete; // Array must have a size, and cannot be stored in a structure.
    	Array(int size); //Throws InvalidSize 
    	//copy constructor
    	Array(const Array& rhs);
    	//move constructor
    	Array(Array&& rhs);
    	//conversion from regular array
    	Array(int* array, int size);
    	// copy assignment operator
    	Array& operator=(const Array& rhs);
    	//move assignment operator
    	Array& operator=(Array&& rhs);
    	//destructor
    	~Array();
    	//index operator
    	int& operator[](int index) const; 
    	//conversion to regular array
    	operator int*() const;
    	//get length of array
    	int getsize() const;
    	//data fields
    	private:
    	int size;
    	int* array;
    };
    
    //Thrower function for input validation
    void IsInputValid(bool condition);
    
    //main
    int main()
    {
    	// size of array
    	int size;
    	//ask for array size
        cout << "How big do you want your array to be?" << endl;
        cin >> size;
        IsInputValid(cin.fail());
        try
        {
        	Array array(size);  
        // get data
        cout << "Enter the numbers you'd like to store: " << endl;
        for (int i = 0; i < array.getsize(); i++)
        {
        	int val;
        	cin >> val;
        	IsInputValid(cin.fail());
        	array[i]=val;
        }
        //display array contents
        cout << "Displaying array contents." << endl;
        for (int i=0; i < array.getsize(); i++)
        cout << array[i] << endl;
        }
        //handle exceptions
          catch (InvalidSize)
        {
        	cout << "Arrays must have at least two objects!" << endl;
        }
        catch (InvalidInput)
        {
        	cout << "Integers only!" << endl;
        }
     //done
     return 0;
    }
    
    //regular constructor
        
      Array::Array(int s): size(s)
      {
      	// make sure array has at least two objects
      	 if (s < 2)
      	throw InvalidSize();
      	//otherwise build the array
      	else
      	{
      		cout << "Building array." << endl;
      		array = new int[size];
      	}
      }
      
    //copy constructor
    Array::Array(const Array& rhs):size(rhs.size), array(new int[size])
    {
    	//delegate to copy assignment operator
    	*this = rhs;
    } 
    
    //move constructor
    Array::Array(Array&& rhs):array(0)
    {
    	//delegate to move assignment operator
    	*this = move(rhs);
    }
    
    //array conversion constructor
    Array::Array(int* carray, int s):Array(size)
     {
     	for (int i = 0; i < s; i++)
     	array[i] = carray[i];
     }
     
     //copy assignment operator
     Array& Array::operator=(const Array& rhs)   
     {
     	if (this==&rhs)
     	throw;
     	if (size != rhs.size)
     	{
     		size=rhs.size;
     		delete[] array;
     		array = new int [size];
     	}
     	for (int i = 0; i < size; i++)
     		array[i] =rhs.array[i];
     		return *this;
     }
     
     Array& Array::operator=(Array&& rhs)
     {
     	//not checking for self-assignment as rhs is most likely temporary
     	cout << "Moving array." << endl;
     	
     	// reallocate array size if need be
     	if (size != rhs.size)
     	{
     		size=rhs.size;
     	    delete[] array;
     	    array = new int[size];
     	}
     	for (int i = 0; i < size; i++)
     	array[i] = rhs.array[i];
     	delete[] rhs.array;
     	rhs.array=0;
     	return *this;
     }
     
     //destructor
    Array::~Array()
    {
    	//deallocate the wrapped array
    	delete[] array;
    	array = 0;
    }
    
    //index operator
    int& Array::operator[](int index) const
    {
    	//perform bounds checking
    	if (index < 0 || index >= size)
    	cout << "Array index must not be negative or greater then or equal to array size." << endl;
    	else
    	return array[index];
    }
    
    //conversion to regular array
    Array::operator int*() const
    {
    	return array;
    }
    
    //getter for size
    int Array::getsize() const
    {
    	return size;
    }
    
    void IsInputValid(bool condition)
    {
    	if(condition)
    	throw InvalidInput();
    	else
    	return;
    }
    When I enter a letter, the InvalidInput exception doesn't seem to get caught by the corresponding catch, but instead the OS catches it, thus calling terminate(). Any ideas as to why?

    Never mind...try block didn't cover the first call of IsInputValid().

  13. #13
    Join Date
    Apr 1999
    Posts
    27,446

    Re: Constructor throws wrong exception

    Quote Originally Posted by TSLexi View Post
    Okay, here's the final version
    Why does your assignment operator throw if it's self assignment? Just make it a no-op.
    Code:
    Array& Array::operator=(const Array& rhs)   
    {
        if (this == &rhs)
           return *this;
       //...
    }
    Also, you need two overloads of operator [] to cover both left and right hand side assignment if [] appears on either side. For example, your array class fails if I did this:
    Code:
    Array a(2);
    //...
    a[0] = a[1];
    The left hand side is non-const, since it changes the internals of the array, but you do not have a non-const overload.

    You also have a bug here:
    Code:
    //array conversion constructor
    Array::Array(int* carray, int s):Array(size)
    Last, what happens if new throws here?
    Code:
    if (size != rhs.size)
    {
       size=rhs.size;
        delete[] array;
        array = new int[size];
     }
    You deleted the array before allocating with new[]. If new[] now throws an exception, your array is in an invalid state, as well as messing up the size variable (it has the size of the rhs array instead of retaining the original value). That's why you should first allocate the memory (create a temp pointer), copy the data to this memory, then do the deletion of the array, and last, assign the temp pointer to the array. If the allocation is successful, then you change the other members, such as size.
    Code:
    if (size != rhs.size)
    {
       int *temp = new int [rhs.size];
       size = rhs.size;  // ok to change now.
    
       // here you now copy the data to temp
       //...
       // now you delete array and assign temp to array
       delete [] array;
       array = temp;
    }
    Also, this is the reason by making the copy constructor do this work, and then delegating the assignment operator to call the copy constructor (and not the other way around, as your code is doing), makes life easier -- you don't need to check for self-assignment, and the code becomes automatically exception safe due to the copy-swap idiom I mentioned earlier.

    Regards,

    Paul McKenzie
    Last edited by Paul McKenzie; January 24th, 2014 at 11:38 AM.

  14. #14
    Join Date
    Dec 2013
    Posts
    75

    Re: Constructor throws wrong exception

    Quote Originally Posted by Paul McKenzie View Post
    Why does your assignment operator throw if it's self assignment? Just make it a no-op.
    Code:
     Array& Array::operator=(const Array& rhs)   
     {
     	if (this == &rhs)
               return *this;
       //...
     }
    Regards,

    Paul McKenzie
    Just remembered that! Haven't written copy assignment operators for a while. BTW, is this basically how std::array is written?

  15. #15
    Join Date
    Jan 2006
    Location
    Singapore
    Posts
    6,391

    Re: Constructor throws wrong exception

    Quote Originally Posted by TSLexi
    is this basically how std::array is written?
    No, std::array is a class template in which the array size is specified at compile time and its copy assignment operator is implicitly declared.
    C + C++ Compiler: MinGW port of GCC
    Build + Version Control System: SCons + Bazaar

    Look up a C/C++ Reference and learn How To Ask Questions The Smart Way
    Kindly rate my posts if you found them useful

Page 1 of 2 12 LastLast

Posting Permissions

  • You may not post new threads
  • You may not post replies
  • You may not post attachments
  • You may not edit your posts
  •  


Windows Mobile Development Center


Click Here to Expand Forum to Full Width

This is a CodeGuru survey question.


Featured


HTML5 Development Center