asked on
myclass::myclass()
// this is a function try block
try
{
// constructor body
}
catch(std::exception const &)
{
// clean-up (and report?)
}
catch( ... )
{
// clean-up
} // <<< automatically re-thrown here
myclass::myclass()
{
// this is NOT a function try block, it is just a try/catch in a function body
try
{
}
catch(std::exception const &)
{
}
catch( ... )
{
}
}
ASKER
ASKER
ASKER
ASKER
#include <iostream>
#include <string>
#include <atlbase.h>
#include <memory>
using namespace std;
class temp{
int* a;
public:
temp():a(nullptr){
a = new int(100);
}
~temp(){
cout <<"Temp destructor called" << endl;
delete a;
}
void Display(){
cout << *a <<endl;
}
};
class MyClassWithMemoryLeak{
private:
temp* ptr1;
temp* ptr2;
temp* ptr3;
MyClassWithMemoryLeak():ptr1(nullptr),ptr2(nullptr),ptr3(nullptr){
throw exception();
ptr3->Display();
}
};
class MyClassWithSmartPointers{
private:
shared_ptr<temp> ptr1;
unique_ptr<temp> ptr2;
unique_ptr<temp> ptr3;
public:
MyClassWithSmartPointers():ptr1(new temp),ptr2(new temp),ptr3(new temp){
throw exception();
ptr3->Display();
}
};
int main()
{
try{
//TODO: Rick, please uncomment below line and comment the line after to see the issue when raw pointers are used
//MyClassWithMemoryLeak* ptrmemleak = new MyClassWithMemoryLeak();
MyClassWithSmartPointers* ptr = new MyClassWithSmartPointers();
}
catch(exception& ex)
{
cout << ex.what() << endl;
}
return 0;
}
#include <iostream>
#include <string>
#include <atlbase.h>
#include <memory>
using namespace std;
class temp{
int* a;
public:
// RX: So, this isn't unsafe because you are allocating only one resource,
// the issue is that if you added another pointer with heap allocation
// if the second allocation failed the first (pointer a) will never be
// deleted because the destructor is never run. If you were to use
// smart pointers for both, the destructor of the smart pointers would
// take care of deleting (or not) as necessary.
temp():a(nullptr){
a = new int(100);
}
~temp(){
cout <<"Temp destructor called" << endl;
delete a;
}
void Display(){
cout << *a <<endl;
}
};
class MyClassWithMemoryLeak{
private:
temp* ptr1;
temp* ptr2;
temp* ptr3;
// RX: none of the pointers are allocated memory, but if they were and an
// exception was thrown before the constructor finished you'd have the
// potential for a memory leak because the destructor will never be
// called.
MyClassWithMemoryLeak():ptr1(nullptr),ptr2(nullptr),ptr3(nullptr){
throw exception();
ptr3->Display();
}
};
class MyClassWithSmartPointers{
private:
shared_ptr<temp> ptr1;
unique_ptr<temp> ptr2;
unique_ptr<temp> ptr3;
public:
// RX: This is nice and safe, because if the constructor fails the smart
// pointer destructors (although the destructor for this class isn't
// called any members that have been created up until the exception is
// thrown still have their destructors called) will clean up whatever
// was allocated to them. This is the only sane and safe way to create
// resouces that need to be cleaned up. It should be noted that smart
// pointers are only designed to work with new/delete heap allocs!
MyClassWithSmartPointers():ptr1(new temp),ptr2(new temp),ptr3(new temp){
throw exception();
ptr3->Display();
}
};
int main()
{
try{
//TODO: Rick, please uncomment below line and comment the line after to see the issue when raw pointers are used
//MyClassWithMemoryLeak* ptrmemleak = new MyClassWithMemoryLeak();
MyClassWithSmartPointers* ptr = new MyClassWithSmartPointers();
}
catch(exception& ex)
{
cout << ex.what() << endl;
}
return 0;
}
#include <iostream>
#include <string>
#include <atlbase.h>
#include <memory>
using namespace std;
class temp{
int* a;
public:
temp():a(nullptr){
a = new int(100);
}
~temp(){
cout <<"Temp destructor called" << endl;
delete a;
}
void Display(){
cout << *a <<endl;
}
};
class MyClassWithMemoryLeak{
private:
temp* ptr1;
temp* ptr2;
temp* ptr3;
MyClassWithMemoryLeak():ptr1(new temp),ptr2(new temp),ptr3(new temp){
throw exception();
ptr3->Display();
}
};
class MyClassWithSmartPointers{
private:
shared_ptr<temp> ptr1;
unique_ptr<temp> ptr2;
unique_ptr<temp> ptr3;
public:
MyClassWithSmartPointers():ptr1(new temp),ptr2(new temp),ptr3(new temp){
throw exception();
ptr3->Display();
}
};
int main()
{
try{
//TODO: Rick, please uncomment below line and comment the line after to see the issue when raw pointers are used
//MyClassWithMemoryLeak* ptrmemleak = new MyClassWithMemoryLeak();
MyClassWithSmartPointers* ptr = new MyClassWithSmartPointers();
}
catch(exception& ex)
{
cout << ex.what() << endl;
}
return 0;
}
// Bad
class foo
{
int * a;
int * b;
public:
foo()
: a(new int())
, b(new int()) // if this throws a is never cleaned up
{
// because of the line below the destructor is never called,
// meaning both a and b will leak
throw std::runtime_error("oops");
}
~foo() // not called if foo throws
{
delete a;
delete b;
}
};
// Dangerous, will probably crash on exception
class foo
{
int * a;
int * b;
public:
foo()
try
: a(new int()) // if this throws???
, b(new int()) // this is never intialised
{
throw std::runtime_error("oops");
}
catch(...)
{
if(a) delete a;
if(b) delete b; // if be was never intialised? Dragons be here!
}
~foo() // not called if foo throws
{
delete a;
delete b;
}
};
// Better, but ugly, duplicates code and error prone
class foo
{
int * a;
int * b;
public:
foo()
try
: a(nullptr)
, b(nullptr)
{
a = new int();
b = new int();
throw std::runtime_error("oops");
}
catch(...)
{
if(a) delete a; // test and delete - duplication of destructor code, ugh!
if(b) delete b; // test and delete - duplication of destructor code, ugh!
}
~foo() // not called if foo throws
{
// you could do the deletion in a single function and call it from both
// the constructor catch and the destructor to avoid code duplication, but
// it's still completely unnecessary if you use smart pointers.
delete a;
delete b;
}
};
// This is the only safe way to do things
class foo
{
std::shared_ptr<int> a;
std::shared_ptr<int> b;
public:
// See how simple the code is now we're using smart pointers? Each smart
// pointer will take care of deleting the memory it is allocated when it is
// destructed. If it is never constructed it is never destructed and so it
// will only delete memory that was actually allocated. In other words, if a
// is created but b throws on constructor a is deleted by the smart pointer
// but since b was never constructed its destructor is never called and so it
// won't try to delete unallocated memory. It's automagically, exception safe
// and best of all the code is about as simple as it gets.
foo()
: a(new int())
, b(new int())
{
throw std::runtime_error("oops");
}
};
ASKER
// Never add code to a destructor that throws. If it's unavoidable
// always wrap it in a try/catch block and swallow the exception
// otherwise your app will crash if this destructor is called as part
// of stack unwinding due to another exception.
foo::~foo()
{
try
{
clean_up_code_that_might_throw();
}
catch(std::exception const & e)
{
cerr << e.what() << std::endl;
}
catch(...)
{
cerr << "unknown exception" << std::endl;
}
}
int main()
{
try
{
auto && f = foo(); // this throws on construction
}
catch(std::exception const & e)
{
std::cerr << e.what() << std::endl;
}
}
ASKER
ASKER
ASKER
bool objectallocationfailed = false;
MyClassWithSmartPointers* ptrmyclass = nullptr;
try{
//TODO: Rick, please uncomment below line and comment the line after to see the issue when raw pointers are used
//MyClassWithMemoryLeak* ptrmemleak = new MyClassWithMemoryLeak();
ptrmyclass = new MyClassWithSmartPointers();
}
catch(exception& ex)
{
if(ptrmyclass == nullptr){
//TODO: you can do your recovery
objectallocationfailed = true;
cout<< "Object was not constructed successfull" << endl;
}
cout << ex.what() << endl;
}
//The boolean can be avoided as well
if(objectallocationfailed && (ptrmyclass == nullptr)){
//TODO:Do recovery
cout << "Doing recovery" <<endl;
}
ReadFile::ReadFile(const std::string & filename)
{
FUNC_IN(filename)
std::ifstream file(filename.c_str()); // "file not found" would not throw an exception
std::string line;
while (std::getline(file, line))
{
myarr.push_back(line); // myarr is a member of ReadFile
}
FUNC_OUT(errno)
}
#define FUNC_IN(info) FuncLog::enter( __FILE__, __LINE__, __FUNCTION__, info); try {
#define FUNC_OUT(err) } catch (MyExc & e) { FuncLog::excatch(ex, err); throw; } \
catch (...) { FuncLog::excatch(err); throw; } \
FuncLog::leave( __FUNCTION__, err)
C++ is an intermediate-level general-purpose programming language, not to be confused with C or C#. It was developed as a set of extensions to the C programming language to improve type-safety and add support for automatic resource management, object-orientation, generic programming, and exception handling, among other features.
TRUSTED BY
1) I would think of throwing a a custom exception say FileNotOpen exception from the other member functions when open has not been called before calling other functions of this class. And for these members functions we can define that it may throw FileNotOpen exception so that clients know what exceptions they can expect.
Another option would be to return error code from each member function but then client would need to check the error code after every call to any member function. This is similar to the approach that COM follows returning HR value for every function. Even some windows apis use approach of returning boolean and then use of getlasterror to get the exact error code. But for your requirement exception works well.
2) regarding 2 boolean approach in constructor, I'm not sure if that is needed.
A) boolean for whether memory allocation was successful : If object of my class has been created successfully then it means that memory allocation was successful.
You can always have constructor throw an error if there is any failure during memory allocation. I would ensure that my member variables are smart pointers (either unique or shared based on my need) instead of raw pointers, so that in case of exceptions thrown from constructor the already allocated memory is freed automatically by calling it's destructor since it's a smart pointer.
B) boolean to indicate file is open : you would store a file handle as Class member variable after opening the file, so we can always check on the validity of the file handle instead of needing a boolean to find that out. You can check if file handle is a valid pointer. Initially the file pointer which is class member variable would have been set to nullptr in the member initialization list or constructor before opening the file.
3) you may also want think of thread safety in case your library can be called from different threads of same program and if there could be a shared resource.
Thanks,
Karrtik