-
January 24th, 2014, 03:04 AM
#1
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!
-
January 24th, 2014, 05:46 AM
#2
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. All my code is tested (unless stated explicitly otherwise) with the latest version of Microsoft Visual Studio (using the supported features of the latest standard) and is offered as examples only - not as production quality. I cannot offer advice regarding any other c/c++ compiler/IDE or incompatibilities with VS. You are ultimately responsible for the effects of your programs and the integrity of the machines they run on. Anything I post, code snippets, advice, etc is licensed as Public Domain https://creativecommons.org/publicdomain/zero/1.0/ and can be used without reference or acknowledgement. Also note that I only provide advice and guidance via the forums - and not via private messages!
C++23 Compiler: Microsoft VS2022 (17.6.5)
-
January 24th, 2014, 06:03 AM
#3
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?
-
January 24th, 2014, 06:13 AM
#4
Re: Constructor throws wrong exception
Originally Posted by TSLexi
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.
-
January 24th, 2014, 06:21 AM
#5
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.
-
January 24th, 2014, 06:24 AM
#6
Re: Constructor throws wrong exception
Originally Posted by TSLexi
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
-
January 24th, 2014, 06:30 AM
#7
Re: Constructor throws wrong exception
So, how should I handle copying between arrays of different sizes?
-
January 24th, 2014, 06:40 AM
#8
Re: Constructor throws wrong exception
Originally Posted by TSLexi
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
-
January 24th, 2014, 07:26 AM
#9
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;
}
-
January 24th, 2014, 08:55 AM
#10
Re: Constructor throws wrong exception
Originally Posted by TSLexi
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.
-
January 24th, 2014, 08:59 AM
#11
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();
}
-
January 24th, 2014, 09:26 AM
#12
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().
-
January 24th, 2014, 11:07 AM
#13
Re: Constructor throws wrong exception
Originally Posted by TSLexi
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.
-
January 24th, 2014, 11:10 AM
#14
Re: Constructor throws wrong exception
Originally Posted by Paul McKenzie
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?
-
January 24th, 2014, 11:19 AM
#15
Re: Constructor throws wrong exception
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.
Posting Permissions
- You may not post new threads
- You may not post replies
- You may not post attachments
- You may not edit your posts
-
Forum Rules
|
Click Here to Expand Forum to Full Width
|