some serious error

[G1vDizwU]

Hi guys,

Please have a look at the code below. There should be some serious error in it which I can't find! What about you please?

double* get_from_jack(int* count);  // Jack puts doubles into an array and
                                   // returns the number of elements in *count
vector<double>* get_from_jill();  // Jill fills the vector


void fct()
{
   int jack_count = 0;
   double* jack_data = get_from_jack(&jack_count);
   vector<double>* jill_data = get_from_jill();
   double* jack_high = high(jack_data,jack_data+jack_count);
   vector<double>& v = *jill_data;
   double* jill_high = high(&v[0],&v[0]+v.size());
   cout << "Jill's high " << *jill_high << "; Jack's high " << *jack_high;
   
   // . . .
   
   delete[] jack_data;
   delete jill_data;
}


template<class Iterator>
Iterator high(Iterator first, Iterator last)
                       // return an iterator to the element in [first:last)  
                      // that has the highest value
{
   Iterator high = first;
   for(Iterator p = first; p!=last; ++p)
   if(*high<*p) high = p;
   return high;
}

[chicofeo]

There should be some serious error in it

What are the errors that you are getting?

[G1vDizwU]

I didn't get any one. But it's what the exercise says that "there is one error in it."

[Moschops]

We'd have to know what the functions get_from_jack and get_from_jill do.

[TheIdeasMan]

Hi,

Lines 18 and 19 ? Deleting stuff that isn't yours ? Those variables go out of scope anyway.

Line 18: jack_data is a pointer returned from a function - who knows how it's underlying data was created - with new or not, or indeed whether it was an array?

Line 19: jill_data is a pointer to an STL container, can one do that?

Line 28: Does it matter that high is the same as the template function name?

Why are there pointers to int, double and STL containers?

[Moschops]

There is an implementation of this here
http://people.ds.cam.ac.uk/nmm1/C++/Exercises/Chapter_20/ex_03.cpp
in which the user does have to delete what comes back from get_from_jack (but shouldn't delete what comes back from get_from_jill). It's hideous.

[G1vDizwU]

This is the code:

#include <C:\Users\CS\Desktop\std_lib_facilities_4.h>
using namespace std;

//------------------------------------------------------------------------------

template<class Iterator >
Iterator high(Iterator first, Iterator last)
// return an iterator to the element in [first:last) that has the highest value
{
	Iterator high = first;
	for (Iterator p = first; p != last; ++p)
		if (*high<*p) high = p;
	return high;
}

//------------------------------------------------------------------------------

double* get_from_jack(int* count);  // jack puts doubles into an array
									// and returns the number of elements
									// in *count
vector<double>* get_from_jill();    // Jill fills the vecto
									
//------------------------------------------------------------------------------

void fct()
{
	int jack_count = 0;
	double* jack_data = get_from_jack(&jack_count);
	vector<double>* jill_data = get_from_jill();

	double* jack_high = high(jack_data, jack_data + jack_count);
	vector<double>& v = *jill_data;
	double* jill_high = high(&v[0], &v[0] + v.size());
	cout << "Jill's high " << *jill_high << ";  Jack's high " << *jack_high << endl;
	// ... 
	delete[] jack_data;
	delete jill_data;
}

//------------------------------------------------------------------------------

int main()
{
	fct();
	system("pause");
	return 0;
}

//------------------------------------------------------------------------------

double* get_from_jack(int* count)
{
	if (!count)
		return 0;

	const int n = 10;

	double* arr = new double[n];

	if (arr)
	{
		*count = n;

		for (int i = 0; i < n; ++i)
			arr[i] = i;
	}

	return arr;
}

//------------------------------------------------------------------------------

vector<double>* get_from_jill()
{
	const int n = 10;

	vector<double>* arr = new vector<double>(n);

	if (arr)
	{
		for (int i = 0; i < n; ++i)
			(*arr)[i] = i;
	}

	return arr;
}

//------------------------------------------------------------------------------ 

It runs successfully (without any error).

@Moschops:
What comes back from get_from_jack is a pointer made by new and copied to jack_data. It, to me, is just as jack_data were a pointer fed by new and both should be deleted in fct(), but how to delete a returned value!?

And both of the returned pointers are made inside their functions by new. So why should we delete *arr of get_from_jack but not *arr of get_from_jill? And is it, at all, possible to delete *arr out of its scope?

[Moschops]

It, to me, is just as jack_data were a pointer fed by new and both should be deleted in fct()

That is incorrect. Memory that is allocated using new must be deleted ONCE.

but how to delete a returned value!?

With delete.

And is it, at all, possible to delete *arr out of its scope?

Yes. With delete.


I get the impression that you don't understand pointers. If you understood pointers, you would not have asked either of these questions. You need to stop, and go back, and learn about pointers.

[G1vDizwU]

Oh, I know about pointers!

I know using delete we can delete a reserved memory. I meant for example, in that code how would you delete *arr (that is where in the code)?

And why didn't you answered this :
>> And both of the returned pointers are made inside their functions by new. So why should we delete *arr of get_from_jack but not *arr of get_from_jill?

[TheIdeasMan]

From the same site Moschops linked earlier:

http://people.ds.cam.ac.uk/nmm1/C++/Exercises/index.html

In particular:

they are not always written in a good style, and

I did most of them in some chapters but only a few in others (though I did all that looked as if they needed something new), and

they rarely include the error checking and commenting that the book asks for, and

they haven't been properly tested, and some are deliberately incorrect (to check on failure modes), and

they don't all do the drill or exercise exactly, because I fairly often got bored :-)

Don't do as these examples do - do as the book says!

I would say the examples are quite different to what Bjarne would have written :+) The page was updated last October - so it's not an old web page, it's shame such material is being used to teach with.

For a start, one shouldn't be using new or delete, or use raw pointers at all. There are pointers to vectors because new was used! STL containers store their data on the heap implicitly. I am guessing that is why the jill vector shouldn't be deleted because it's an STL container. Normally they are let to go out of scope naturally (implicitly deleted), but here they are artificially kept alive by returning a pointer from the function.

Trivially, the functions create brand new arrays (as in a different one with the same data) each time they are called.

I am uneasy about delete being used from a different scope. I can imagine it being used at a low level within in a class say, but using it on something created with another general function is rather smelly IMO.

Anyway, it's all messy.

In my mind the whole idea of the STL: is to use it's containers, functions and algorithms and features. Then one doesn't have to worry about memory management at all. The STL is supposed to make life easier and convenient.

Here is a guideline written by Bjarne Stroustrup & Herb Sutter:

https://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines.html#Rr-newdelete

[G1vDizwU]

The book I read and use, first edition, first printing, is of 2008!
There may be issues in it and also many high-level opinions on it again! I'm not sure I got your intention above completely!

Anyway, this Try This is of chapter 20 of this book and this chapter introduces STL to us for the first time.

I got your point on not should get_from_jill be deleted (because of being an STL container).

Now I think what I need is using an exceptions catch for the main() try { for the case of failure of new:

catch (exception& e)
{
	cout << "Standard exception: " << e.what() << endl;
}

And about not using new and delete explicitly, I read the remarks of Biarne and Herb. But here I think I should use them to solve this Try This! :(

Thanks so much for your comments. :-)