finding and fixing errors by using STL techniques

[tomy12]

I'm trying to solve this exercise: (#4)

https://books.google.nl/books?id=We2...page&q&f=false

The section 20.3.1 talks about this code:

#include <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 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 << 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;
}

According to the comments of the two declared functions (line 18 through 21) I defined them like above.

Now, what errors are there in your opinions? And what STL techniques are useful for fixing them please?

[2kaud]

Now, what errors are there in your opinions?

What error(s) is the compiler indicating? What were you expecting?

With VS2015 it displays 9 for both Jack and Jill's high.

[2kaud]

Now that I've looked at the code, what the book is alluding to is not a compiler error but a potential run-time error. get_from_jill() and get_from_jack both use new() to obtain memory. However the default version of new throws an exception if memory could not be allocated. But the functions assumes that new() returns 0 on error which is incorrect.

There are 2 possible solutions. One is use a different version of new that does return 0 on failure (nothrow), the second is to use exceptions which were covered in chapter 19.

For nothrow, consider

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

then arr will be 0 if an error occurs.

In this case though, in fct() get_from_jack() and get_from_jill() could return 0 and so within fct() the return values need to be checked to ensure they are not 0. And as fct() doesn't return anything, how does the caller of fct() (in main()) know that a problem has occurred? fct() would need to return a value which would have to checked in main(). A lot of checking for something that probably won't occur but is major if it does!

However, it is much better to use the c++ way of doing things and use exception handling. As normally new doesn't return 0 there is no need to check for a 0 returned so get_from_jill() becomes

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

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

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

	return arr;
}

and similarly for get_from_jack()

Then within main() you can use a try..catch exception block around the call to fct() to catch the exception. You could also create your own exception class with an exception for jill and an exception for jack so that in fct() you put separate try-catch around get-from-jill and get-from-jack and throw a different exception for each. Then within main() you could have different catch clauses for jill exception and jack exception so that you knew which caused the error in fct().

I'll leave this as an exercise as you're just covered exceptions in chapter 19.

The book is alluding to the general technique of using exceptions in code to trap and deal with potential run-time errors.

[2kaud]

You asked this same question here http://forums.codeguru.com/showthrea...ight=jack+jill 31 May 2016! It's not that long ago to have forgotten!

[tomy12]

Yes, I read what you wrote and they are completely reasonable. But there is some case!

Let me first talk about your other (good) explanations. Yes, I asked it days ago but that was a Try This, this one is the end of the chapters' exercise. And, the remedy you offered spots the issues you talked about well. To be honest, your strategies are useful and SHOULD be used in the code. But one question here, is the strategy you offered (catching) part of STL Techniques? If yes, so it may have been what Stroudtrup meant to be used for the code.

And the case: Those two functions (get_from_jack and get_from_jill) are declared in the code written in that section (of the book). I think the errors (meant by the author) are out of them because they have not been defined there. It's just a guess.

PS: the code runs for me without errors. IDE is VS 2015

[2kaud]

But one question here, is the strategy you offered (catching) part of STL Techniques?

Not specifically. It is part of general c++ good practice programming for production code. There is a difference between example code (such as posted to forums like this), test code to see how something works and production code. Production code needs to be robust and needs to deal with any and every error that can occur. c++ exceptions are part of the answer to making production code robust. It is also a useful technique to dealing with errors that are not frequent otherwise the code can be littered with if statements (try recoding the jack/jill example with nothrow on the new and deal with a return value of 0 everywhere)! Note however that there is a run-time overhead if an exception is triggered so they aren't normally used for cases where an error is likely.

[2kaud]

To provide some critique of the code, given the functions get_from_jack() and get-from_jill(), fct() could be coded as (excluding exceptions etc)

	int jack_count = 0;
	auto jack_data = get_from_jack(&jack_count);
	auto jack_high = high(jack_data, jack_data + jack_count);

	auto jill_data = get_from_jill();
	auto jill_high = high(jill_data->begin(), jill_data->end());

using auto instead of an explicit type name when the compiler knows the type of the variable. Also using begin() and end() for the vector instead of obtaining the address of the first element and then computing the address of he last.

However IMO a more serious critique is that both get_from_jill() and get_from_jack() return an address allocated within the function and expect the caller to deal with freeing the memory. This is where memory problems occur with either multiple free or memory not freed at all. When looking at allocating memory, first look at using unique_ptr (see http://www.cplusplus.com/reference/memory/unique_ptr/) and then shared_ptr (see http://www.cplusplus.com/reference/memory/shared_ptr/) and if these don't fit then consider a RAII class design (look it up!).

[2kaud]

Without using new (and hence no free is required) a 'better' c++ coding (without exception handling) could be

#include <vector>
#include <iostream>
#include <memory>

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;
}

auto get_from_jill()
{
	const int n = 10;

	auto arr = make_unique<vector<double>> (n);

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

	return arr;
}

auto get_from_jack(int *const count)
{
	const int n = 10;

	if (count != nullptr)
		*count = n;

	auto arr = make_unique<int[]>(n);

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

	return arr;
}

void fct()
{
	int jack_count = 0;
	auto jack_data = get_from_jack(&jack_count);
	auto jack_high = high(jack_data.get(), jack_data.get() + jack_count);

	auto jill_data = get_from_jill();
	auto jill_high = high(jill_data->begin(), jill_data->end());

	cout << "Jill's high " << *jill_high << ";  Jack's high " << *jack_high << endl;
}

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

[jlb1]

However IMO a more serious critique is that both get_from_jill() and get_from_jack() return an address allocated within the function and expect the caller to deal with freeing the memory.

There is a stipulation in these exercises that state that both of those functions can not be modified. From Chapters 20.1 and 20.1.1:
"The assumption is that the data is stored on the free store and that we should delete it when we are finished using it. Another assumption is that we can's rewrite Jack's and Jill's code, or wouldn't want to."
And in the next paragraph:
Clearly, this is a somewhat simplified example, but it is not dissimilar to a vast number of real-world problems. If we can handle this example elegantly, we can handle a huge number of common programming problems. The fundamental problem here is that we don't control the way in which our "data suppliers" store the data the give us. It's our job to either work with the data in the form in which we get it or to read it and store it the way we like better."

So yes these methods leave a lot to be desired but we need to deal with these problems.

[tomy12]

Thank you all for the answers especially thanks to 2Kaud.

[tomy12]

Another question please

Here in the code vector<double>& v = *jill_data;, why not using vector<double> v = *jill_data; instead of it? That is why do we need to use a reference to vector<double> please?