Don't catch an exception if you don't know how to handle it

closed account (jvqpDjzh)
Is it a good programming practice? I have read many posts about the topic, but nobody gives concrete examples.
Let's say that we are trying to catch a bad_alloc exception:
1
2
3
4
5
6
7
	try{
		 p = new int[4*1000000000];
                 //ok, this definitely will make my program crash.	
	}catch(bad_alloc ex){
		delete[] p;
		cout << "Error: "<<ex.what() << endl;	
	}
//this snippet doesn't help
//How exactly do you handle a situation like this? If you try to allocate so much memory, if new throws a bad_alloc, what exactly can we do to make our program continue its normal execution and maybe try to reallocate memory again and don't make the program crash again?
If new throws bad alloc, no memory allocation is made, so you should not delete p;
what exactly can we do to make our program continue
That depends on concrete example: why is bad_alloc thrown?
Let's say it tries to allocate character array to load whole file in it. User provides file name, it loads it, manipulates it et cetera. Now if file is too large attempt to allocate an array for it is going to fail. In this case you might write a message ("Cannot allocate memory to load file. Probably file is too large.") and do the same that you do if file is not found or inaccessible: either terminate program or suggest to enter new name.

Is it a good programming practice?
Yes. As long as you know where and how you should handle exceptions. If you catch an exception and do not handle it properly, you rob higher level code from ability to handle it. Calling code can have context current function does not have.

For example you might wrap c-style array in RAII container (like postponed dynarray). You provide array size in constructor and use it as normal array without fear of memory leaks. Internally it uses new to allocate array. It can throw an bad_alloc. Should it handle it? No: it does not know how. Should calling code know about it? Does it expect it and have fallback mechanisms t won't be able to use if you intercept exception?

Notice that capturing exception and rethrowing it is valid strategy if you want, say, log all allocation failures.
closed account (jvqpDjzh)
Thank you MiiNiPaa, as usual, very helpful! But could you do simple concrete examples about what you are talking? Or tell where can found these examples..
I do not process exceptions in loadFile because I do not know how that function will be used. In this case it should output error to user. In other cases it might be nessesary to write message in logfile. Sometimes it is so serious that it should terminate program.
Let code which knows what to do (in my case process()) handle it:
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
#include <iostream>
#include <fstream>
//Includes portable function fileSize 
//which returns size of file in bytes
#include <portable.h>

char* loadFile(const std::string& filename)
{
    if(!portable::fileExist(filename))
        throw IOexception("File not found: " + filename);
    std::ifstream input(filename, std::ios::it | std::ios::binary);
    if(!input)
        throw IOexception("Error opening file " + filename;
    char* filecontent = new char[portable::fileSize(filename)];
    //Load file
    //IOexception can be thrown here too
}

bool process(const std::string& filename)
{
    char* file = nullptr;
    try {
        file = loadFile("Hello.blob");
        //Some other manipulation
    } catch(IOeception& ex) {
        std::cout << ex.what() << '\n';
        delete[] file;
        return false;
    } catch(std::bad_alloc& ex) {
        std::cout << "insufficient memory to load file " << filename << '\n';
        return false;
    }
    delete[] file;
    return true;
}

int main()
{
    std:string file;
    std::cout << "Enter file to process: ";
    std::cin >> file;
    while(!process(file)) {
        std::cout << "Error processing file, enter another file name or \"$exit\" to exit\n";
        std::cin >> file;
        if (file == "$exit") break;
    }
}

1
2
3
4
5
catch(IOeception& ex) {
        std::cout << ex.what() << '\n';
        delete[] file; //wrong
        return false;
    }
cmiiw: file is either NULL or invalid (I take NULL)
If the function throws an exception it does not returns a value to be assigned to the 'file' variable, so you've got a memory leak.
Oh. Yes. You should delete array inside loadFile before throwing exception.
And that is why you should avoid raw new/delete. Use RAII or smart pointers.
1
2
3
4
5
catch(IOeception& ex) {
        std::cout << ex.what() << '\n';
        delete[] file; //wrong
        return false;
    }

cmiiw: file is either NULL or invalid (I take NULL)
If the function throws an exception it does not returns a value to be assigned to the 'file' variable, so you've got a memory leak.

More accurately, if the function throws an exception then the assignment to 'file' never happens. Fortunately, it's initialized to nullptr so the delete[] is safe.

For the same reason, the original post's code may have undefined bahavior, depending on the value of p coming into the code:
1
2
3
4
5
6
7
try{
	 p = new int[4*1000000000];
         //ok, this definitely will make my program crash.	
}catch(bad_alloc ex){
	delete[] p;  // p was not assigned
	cout << "Error: "<<ex.what() << endl;	
}

Is it a good programming practice?

Probably not. When you're out of memory, you have to assume that any future attempt to allocate memory will fail. I realize this isn't true for the example you gave, but in real code, it is. So cout<<"Error:" might fail (it might try to allocate memory for buffering) and ex.what() might fail for the same reason.

The only way I've seen to gracefully handle out-of-memory conditions in anything but trivial programs is this:
- allocate some amount of "emergency memory" at the beginning of the program.
- If you run out of memory, the first thing you do is free the emergency memory. This ensures that some future allocations will work.
- Save the users data and exit.


More accurately, if the function throws an exception then the assignment to 'file' never happens. Fortunately, it's initialized to nullptr so the delete[] is safe.
You make it sound like memory leak does not happens. Indeed, assigment does not happens and function does not return pointer too, so both statements are valid, however we still have memory leak.

Is it a good programming practice?
Probably not
I am under impression that OP wants to know in general and that example was all he could thought about quickly.

bad_alloc has two leading causes: You run out of memory and you tried to allocate too much. We focused on latter. In former case, as you pointed, we are probably doomed anyway.

Herb Sutter article about it: http://www.gotw.ca/publications/mill16.htm
Last edited on
closed account (jvqpDjzh)
In summary, in your example, there are 2 exceptions that can be thrown and caught, which are the one that you created and the bad_alloc one, that can be thrown by new in the function loadFile, right?

So, if an exception is thrown no memory is allocated, therefore we shouldn't use delete[] ptr; to prevent memory leaks?

Another thing, could we use the seekg and tellg to find the length of the file:
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
inline int getFileSize(const string& fileName)
{	
       if(exists_test0(fileName)){
	ifstream file (fileName.c_str(), ios::binary);
        //what exactly is end? 
        //I know it is of type: ios_base::seekdir, is this a pointer, like iterators?
	file.seekg(0, file.end);
	
	int length = file.tellg();

	file.seekg(0, file.beg);
        //why do we do this? 
//To have the pointer to the file to the beginning of the same the next time  we open it?
         file.close();
	return length;
        } else { return -1;}
        
}
Last edited on
therefore we shouldn't use delete[] ptr;
My example wasn't full because I got tired of writing it. There could be IOexception thrown in process() function too. That is why I initialized pointer to nullptr: to be able to delete it and do not care about possible fails in assigment.
You can safely delete a null pointer. The fact that address of allocated buffer was never assigned to ptr leads to leak, not delete[]. You have to delete buffer in load file before you throw exception (it can happens in part replaced by comment)

could we use the seekg and tellg to find the length of the file
Yes
what exactly is end?
Unspecified. Usually it is an enumeration value (integer, say, 2)
//why do we do this?
//To have the pointer to the file to the beginning of the same the next time we open it
It this case it is useless. If you would like to continue working with this file before closing it, this line "rewinds" file allowing to read from the beginning.
Consider an example where we do a web search, but if we don't have internet we search a database. Another problem with the web search is that the site might be down. Fortunately, we have a list of sites that we can try. The function that goes through the list is not able to handle the situation where the internet is down, so it simply ignores that exception. The function that makes the decision between an internet response or something from the database does need to know if there is no connection, so that is where the exception is caught.

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
std::string internet_query( const std::string& site, const std::string& query )
{
  std::string result = magical_function( site, query );

  if ( result == "No Internet" ) throw NoInternetException();
  if ( result == "Site down"   ) throw SiteDownException();

  return result;
}

std::string internet_answer( const std::string& query )
{
  static std::vector<std::string> sites = { /* best sites */ };
  for ( auto& site : sites )
  {
    try
    {
      return internet_query( site, query )
    }
    catch ( const SiteDownException& ex )
    {
      // maybe remove the site from the vector
    }
  }
}

std::string database_answer( const std::string& query )
{
  return magical_function( query );
}

std::string answer( const std::string& query )
{
  try
  {
    return internet_answer( query );
  }
  catch ( const NoInternetException& ex )
  {
    return database_answer( query );
  }
}
Last edited on
closed account (jvqpDjzh)
Possible description:

So you call the function answer, which calls the internet_answer(), which calls the internet_query(), which calls magical_functions(), and depending on what this function returns 2 type of exceptions can the thrown: NoInternetException and SiteDownException. Exceptions of type SiteDownException can be caught in internet_answer and if the exception is not of type SiteDownException but of type NoInternetException, internet_answer will return the control to answer(), which can caught the NoInternetException and call the database_answer() function.


Another thing:
I have seen somewhere that when you throw an exception instead of using just the contructor of the class to throw the exception, it is used the operator new, creating on the volley an object, using your example:
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71
72
73
74
75
76
77
78
79
80
81
82
83
84
85
86
87
88
89
90
91
92
93
94
95
96
97
98
99
100
#include <iostream>
#include <string>
#include <exception>
#include <vector>
using namespace std;

class NoInternetException : public exception
{
public:
    const char* what() const throw(){
        return "What the heck?";
    }
};

class SiteDownException : public exception
{
};

string magical_function(string site="", string query="")
{
    return "No Internet";
}

string internet_query( const string& site, const string& query )
{
    string result = magical_function( site, query );

    if ( result == "No Internet" ) throw NoInternetException();
     //if ( result == "No Internet" ) throw new NoInternetException();
    //This makes the program crash:
    //creating on the volley an object. It seems that we can't do it!
    //Maybe I have seen this in JAVA
    //What are the differences?
    if ( result == "Site down"   ) throw SiteDownException();

    return result;
}

string internet_answer( const string& query )
{
    static vector<string> sites = {"google", "youtube", "facebook"};

    //Can you tell me a good guide to this new for syntax (is it from C++11)?
    //Actually, I can understand what it is doing
    for ( auto& site : sites )
    {
        try
        {
            return internet_query( site, query );
        }
        catch ( const SiteDownException& ex )
        {
            // maybe remove the site from the vector
        }
    }
}

string database_answer( const string& query )
{
    return magical_function( query );
}


string answer( const string& query )
{
    try
    {
        return internet_answer( query );
    }
    catch ( const NoInternetException& ex )
    {
        //the exception is caught and also the next statement is executed.
        cout << "Error: "<<ex.what()<<endl;
        
        //this returns "No Internet" from the magical_function
        return database_answer( query );
       
    }
}


int main()
{
    string str = answer("www.youtube.com");
    cout << "string returned: "<<str<<endl;//No Internet

    return 0;
}
//I have seen you more experts dudes using always std:: 
//before any name from the std namespace.
//I have seen this post 
//(http://stackoverflow.com/questions/1452721/why-is-using-namespace-std-considered-bad-practice),
//and here they explain why it's better to use std::stdname 
//instead of "using std::cout" or "using namespace std"
//But, in my opinion, at least the names from the std namespace 
//shouldn't be preceded always by std:: 
//We use them too much, I must say always, so I suggest, 
//just for the std namespace to remove 
//and stop writing std:: before every little std:: name. 
//Please Bjarne, change this! 
Last edited on
I have seen somewhere that when you throw an exception instead of using just the contructor of the class to throw the exception, it is used the operator new,
Operator new still calls constructor BTW.
operator new returns a pointer, which gets thrown. We do not capture any pointers to exception, so it goes uncaptured leading to program termination.
You should capture it like: catch (const NoInternetException* ex )

In C++ you should prefer throwing by value and capturing by reference.

Can you tell me a good guide to this new for syntax (is it from C++11)?
This is range-based for. Introduced in C++11
http://www.cprogramming.com/c++11/c++11-ranged-for-loop.html

//just for the std namespace to remove
//and stop writing std:: before every little std:: name.
There was time where everything was in global namespace. It was a grim times, you could not write a hello world without encountering name clashing.
Seriously. You will not beleve what names are used by standard library. Having them all in global namespace means that you will not be able to ever calss or variable named, say, count. Or distance. It might appears to work, but next you need something else, you include one more file and you spend rest of the day searching for source of hard to find error.
You might use using declaration instead of using directive to bring names you need into global namespace:
1
2
3
4
int main()
{
    using std::cin;
    using std::endl; //We will output things often 
sometimes you will need to use using declaration:
1
2
3
4
5
6
7
8
template <typename T>
void foo(T a, T b)
{
    using std::swap;
    //code...
    swap(a, b); //Allowing for ADL to kick in
    //even more code...
}
closed account (jvqpDjzh)
operator new returns a pointer
Yes, I could have thought about that. Yes, right: throwing by value, catching by reference. This is because manage pointers are definetily more complicated?! And catching by value would mean making a copy...
I turned the code I posted into an actual program, maybe it helps you see the flow:
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71
72
73
74
75
76
77
78
79
80
81
82
83
84
85
86
87
88
89
90
91
92
93
94
#include <iostream>
#include <string>
#include <vector>

struct NoInternetException
{
  std::string what( void ) const
  {
    return "No Internet Exception";
  }
};

struct SiteDownException
{
  std::string what( void ) const
  {
    return "The site is down";
  }
};

std::string magical_function( void )
{
  // its magical
  static int count = 0;
  switch( ++count )
  {
    case 1: return "Site down";
    case 2: return "internet search successful";
    case 3: return "No Internet";
    case 4: return "database search successful";
  }
  return "This doesn't happen in this program";
}

std::string internet_query( void )
{
  std::string result = magical_function();

  if ( result == "No Internet" )
    throw NoInternetException();

  if ( result == "Site down"   )
    throw SiteDownException();

  return result;
}

std::string internet_answer( void )
{
  static std::vector<std::string> sites =
    { "cplusplus", "stackoverflow" };
  for ( auto& site : sites )
  {
    try
    {
      std::cout << "searching: " << site << '\n';
      return internet_query();
    }
    catch ( const SiteDownException& ex )
    {
      std::cout << ex.what() << ": " << site << '\n';
    }
  }
}

std::string database_answer( void )
{
  return magical_function();
}

std::string answer( void )
{
  try
  {
    return internet_answer();
  }
  catch ( const NoInternetException& ex )
  {
    std::cout << ex.what() << ": searching database\n";
    return database_answer();
  }
}

int main( void )
{
  // this call to answer finds out that cplusplus is down
  // and then finds an answer at stackoverflow
  std::cout << answer() << '\n';

  // this call to answer finds out that the internet is down
  // and then searches the database
  std::cout << answer() << '\n';
  return 0;
}
searching: cplusplus
The site is down: cplusplus
searching: stackoverflow
internet search successful
searching: cplusplus
No Internet Exception: atempting database search
database search successful

closed account (jvqpDjzh)
I have understood a possible way of using your methods. Didn't you see my previous post?
Topic archived. No new replies allowed.