custom exception class destructor

[positivelyskewed]

I have this code that creates a custom exception class which has private data members of type const char*. I want to delete those pointers in the destructor, but since the exception is thrown inside the Price object's code, the destructor seems to be getting called before catch block in the main function. I'm new to this, so if anyone could point me in the right direction, I'd really appreciate it.

//CCException.h
#pragma once
#include <iostream>
using namespace std;

class CCException
{
public:
	CCException(const char* m, const char* f);
	~CCException(void);
	void PrintMessage() const;
private:
	 const char* message;
	 const char* function;
};

//CCException.cpp
#include "CCException.h"


CCException::CCException(const char* m, const char* f)
{
	message = m;
	function = f;
}


CCException::~CCException(void)
{
	//delete [] message;
	//message = NULL;
}

void CCException::PrintMessage() const
{
	cout <<"Error in "<< function << ": " << message << endl;
}

//price.h
#pragma once
#include "CCException.h"

class Price
{
public:
	const enum Term {Short, Medium, Long};
	Price(const int &n);
	~Price(void);
	const double getPrice(const int &i) const;
	void setPrice(const double &d, const int &i);
	void setMR(const double &d, const Term &t);
	void setVol(const double &d, const Term &t);
private:
	double* forwardcurve;
	double a[3];
	double v[3];
	int count;
};

//price.cpp
#include <time.h>
#include "price.h"
#include "CCException.h"

Price::Price(const int &n)
{
	count = n;
	for (int i = 0; i < 3; i++) {
		a[i]=0;
		v[i]=0;
	}
	srand(time(NULL));
	forwardcurve = new double[n];
	for (int i = 0; i < n; i++) {
		forwardcurve[i] = (double)(rand() % 1000) / 100;
	}
}

Price::~Price(void)
{
	delete [] forwardcurve;
	forwardcurve = NULL;
}

const double Price::getPrice(const int &i) const
{
	if (i >= 0 && i < count)
		return forwardcurve[i];
	else {
		throw CCException("Index out of range.","Price::GetPrice");
	}
}

void Price::setMR(const double &d, const Term &t)
{
	if (d >= 0)
		a[(int)t] = d;
}

void Price::setVol(const double &d, const Term &t)
{
	if (d >= 0)
		v[(int)t] = d;
}

void Price::setPrice(const double &d, const int &i)
{
	if (d > 0 && i < count)
		forwardcurve[i] = d;
	else{
		throw CCException("Index out of range.","Price::SetPrice");
	}
}

//main.cpp
#include "price.h"
#include "CCException.h"
#include <iostream>

using namespace std;

int main()
{
	int n = 20;
	Price p = Price(n);
	char buffer[100];

	try {

                //This throws an exception when n=20.
		for (int i = 0; i < n+1; i++)
			cout << p.getPrice(i) << endl;
	}

	//Error catching code.
	catch (const char *e){
		cout << e << endl;
	}
	catch(const CCException e){
		e.PrintMessage();
	}
	catch(...){
		cout << "Unknown Error"<<endl;
	};

	cin.getline(buffer,100);
};

[Chris_F]

I think it is generally considered bad practice to create your own exception classes from scratch, as opposed to publicly deriving them from std::exception. You should probably consider using std::string objects instead of char*s. Also, n being equal to 20 hardly seems like an exceptional case. You should make sure you are using exceptions for the right reasons.

The other mistake you are making is trying to delete the char* in the first place. You only delete things that are created with new.

[positivelyskewed]

Thanks for all of the advice. This isn't really anything I'm going to actually use, I'm just sort of testing out what everything in C++ does.

[positivelyskewed]

Also, n being equal to 20 hardly seems like an exceptional case. You should make sure you are using exceptions for the right reasons.

I mistyped. I meant when i = 20. i = 20 is an exceptional case because the forwardcurve array only has 20 elements, so trying to get forwardcurve[20] would be out of the bounds. Is it not good practice to throw exceptions in this case? What should one do?

Thanks again!

[D_Drmmr]

I have this code that creates a custom exception class which has private data members of type const char*. I want to delete those pointers in the destructor, but since the exception is thrown inside the Price object's code, the destructor seems to be getting called before catch block in the main function. I'm new to this, so if anyone could point me in the right direction, I'd really appreciate it.

That's because you catch by value and your exception class is not safely copyable. You should always catch by const reference and you should always make your classes safely copyable or disable copying by declaring the copy constructor and copy assignment operator private (or use the = delete syntax from C++0x if your compiler supports this).

[Paul McKenzie]

Thanks for all of the advice. This isn't really anything I'm going to actually use, I'm just sort of testing out what everything in C++ does.

Then you should learn the proper practices. A C++ program can be written elegantly, or without any coherence or thought. Either case will produce a compilable program that can work, but what about maintenance, enhancements, and debugging issues?

I want to delete those pointers in the destructor,

Learn RAII and use objects that are smart enough to copy and destroy themselves automatically.

For example, a std::string instead of char* would have alleviated the problem, since a std::string knows how to copy itself and destroy itself automatically without you having to write any code. A char* is just a "dumb" pointer that you now have to manage.

Here is a simplified version of the mess you got yourself into with your exception class:

class foo
{
     char *ptr;

     public:
     foo()
     {
           ptr = new char [10];
     }

     ~foo()
      { 
          delete [] ptr;
      }
};

int main()
{
   foo f1;
   foo f2 = f1;
   foo f3;
   f3 = f1;
}

The code above has a memory leak, and a triple deletion error, where you're deallocating the same pointer three times on exit.

If this class were written to take care of all of these problems, the hard way to do it would be this:

#include <cstring>

class foo
{
     char *ptr;

    public:
    foo()
    {
           ptr = new char [10];
    }

     ~foo()
     { 
         delete [] ptr;
     }

     foo(const foo& rhs)
     {
        ptr = new char[10];
        memcpy(ptr, rhs.ptr, 10);
     }

     foo& operator =(const foo& rhs)
     {
          if ( &rhs != this )
          {
              char *temp = new char [10];
              memcpy( temp, rhs.ptr, 10 );
              delete [] ptr;
              ptr = temp;
          }
          return *this;
     }
};

Now the main() program will work correctly.

Or you can do it the easy way:

#include <string>

class foo
{
    std::string ptr;

    public:
    foo() : ptr(10, '\0') { }
};

The code above essentially does the same thing as the code that had the char*. Now which one is easier to maintain? But the more important question is this -- why is the second code easier to maintain? What in the second example makes a user-defined destructor, assignment operator, and copy constructor unnecessary?

Regards,

Paul McKenzie

[monarch_dodra]

That's because you catch by value and your exception class is not safely copyable. You should always catch by const reference and you should always make your classes safely copyable or disable copying by declaring the copy constructor and copy assignment operator private (or use the = delete syntax from C++0x if your compiler supports this).

Technically, even when you catch an exception by reference, the compiler still uses pass by value. This is due to the fact that a catch never returns control to the caller, and is thus responsible for clean-up.

That said, when you catch by value, chances are you copy the the exception... twice.

Meyers gives all the details in one of his books, can't remember which. Cline also states in his FAQ, that an exception MUST be copy constructible... even if the copy constructor is never called.

[positivelyskewed]

Then you should learn the proper practices. A C++ program can be written elegantly, or without any coherence or thought.

Exactly what I'm trying to figure out! Thanks for the tips. I appreciate all of the criticism because I really want to learn how to write good code, not just code that works.

[positivelyskewed]

What in the second example makes a user-defined destructor, assignment operator, and copy constructor unnecessary?

a std::string knows how to copy itself and destroy itself automatically without you having to write any code.

That was a very useful example. I actually didn't realize that the first code had that triple deletion thing going on. Looks like I have a lot of reading to do...

[Paul McKenzie]

Exactly what I'm trying to figure out! Thanks for the tips. I appreciate all of the criticism because I really want to learn how to write good code, not just code that works.

Well, with C++, you start out with components that work (for example std::string), and put together a program from workable smaller parts. At least, that is the modern way to learn C++. Unfortunately, most courses do not teach C++ this way, and instead teach 'C' programming with some C++ syntax thrown in.

You still have to write the logic to put these components together to create a program, but the foundational blocks you're using must be sound. With the example you had, you were struggling with string data, when there was no need to -- C++ handles string data easily if you use the right standard components.

The same thing with dynamic arrays, linked lists, and other aspects. There is no need to write your own if the ones available to you from the standard library (vector, list, deque, set, map, etc.) are adequate (and for a vast majority of C++ programmers, they are adequate, if not superior to whatever the programmer could write themselves). The goal being that you don't want to be in the maintenance business maintaining this "low-level" code, which means spending time debugging it if something is wrong. Instead, you give more time in developing the program you're creating, and not creating mini linked-list or string classes before even getting started.

So if you use the jigsaw puzzle as an analogy to writing a program, the C++ thinking shifts from having to create each jigsaw puzzle piece (your attempt at using char*) into how to efficiently and elegantly put the jigsaw puzzle together from the pieces that are already created.

You will get to the point where you may have to write code such as the first example of the foo() class, but make those times rare. I can't remember the last time I had to write code like the first foo() class in a real application, and I've been doing this for over 20 years now.

Regards,

Paul McKenzie