C++

Forum

 
Array data loss after return from function

Pages: 12
fatirishman53 (34)
I searched half of the day, yesterday,trying to figure this out. If anyone can help I would appreciate it very much. I have an object that I use to store data. Two of its members are dynamic arrays. When passed (by reference) to a function used to fill the arrays with data, everything works fine. However, only the 0th index of the arrays remain populated after the function call. I realize that it is quite possible that I'm just an idiot, but I thought I would ask, just in case I'm not.

parameters.cpp // class reference
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
 
#ifndef parameters_H
#define parameters_H

using namespace std;

class parameters
{
public:
	double k;
	double c;
	double a;
	double vci;		// Cut-in velocity
	double vr;		// Rated velocity
	double vco;		// Cut-out velocity
	double vavg;	// Average velocity
	double area;	// Turbine blade area
	double airD;	// Air density
	double pr;		// Rated power
	double inc;		// Velocity iteration increment
	double CF;		// Capacity factor
	double pavg;	// Average power

	double *dataPower;
	double *dataPDF;

	parameters()
	{
		k = 0;
		c = 0;
		a = 0;
		vci = 0;		// Cut-in velocity
		vr = 0;		// Rated velocity
		vco = 0;		// Cut-out velocity
		vavg = 0;	// Average velocity
		area = 0;	// Turbine blade area
		airD = 0;	// Air density
		pr = 0;		// Rated power
		inc = 0;		// Velocity iteration increment
		CF = 0;		// Capacity factor
		pavg = 0;	// Average power
	}

	void arrCreate(int arrSize)
	{
		dataPower = new double[arrSize];
		dataPDF = new double[arrSize];
	}
	
	~parameters()
	{
		delete [] dataPower;
		delete [] dataPDF;
	}
};

#endif



Functions.h // functions and sub-functions
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
 
#include <cmath>
#include "parameters.cpp"

void populate(parameters &);
double fV(parameters &, double);
double CF(parameters &);
double incGamma(double, double);
double pGen(parameters &, double);

using namespace std;

void populate(parameters &param)
{
	int index = 0;

	for (double V = 0; V <= param.vco; V += param.inc)
	{
		if (V < param.vci)
		{
			param.dataPower[index] = 0;
			param.dataPDF[index] = fV(param, V);
		}
		else
		{
			param.dataPower[index] = pGen(param, V);
			param.dataPDF[index] = fV(param, V);
		}
	}
}

double fV(parameters &param, double V)
{
	return param.k / param.c * pow(V / param.c, param.k - 1.0) * exp( - pow(V / param.c, param.k) );
}

double CF(parameters &param)
{
	return pow(param.vr, -3.0) * 
			incGamma(param.a, param.vr) - 
			incGamma(param.a, param.vci) + 
			exp( - pow(param.vco / param.c, param.k) ) - 
			exp( - pow(param.vr / param.c, param.k) );
}

double incGamma(double a, double x) // Consider using recursion
{
	double gammaSum = 0;
	for(int n = 0; n <= 100; n++)
	{
		double denom = 1;
		for(int nInc = 0; nInc <= n; nInc++)
		{
			denom *= (a + nInc);
		}

		gammaSum += ( pow(x, n) / denom );
	}

	return gammaSum * pow(x, a) * exp(-x);
}

double pGen(parameters &param, double V)
{
	return param.pr * pow(V, 3) / pow(param.vr, 3);
}


main.cpp // main and output functions
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
 
#define _USE_MATH_DEFINES
#include <cmath>
#include <iostream>
#include <string>
#include <cstdlib>
#include "Functions.h"
#include "parameters.cpp"

void output(parameters&, int);

using namespace std;

int main()
{
	parameters param;
	param.vci = 0;
	param.vr = 7.0;
	param.vco = 16.0;
	param.vavg = 7.0;
	param.area = 10.869;
	param.airD = 1.2;
	param.pr = 2.1;
	param.inc = 0.5;

	param.k = 2.0;
	param.c = 2 * param.vavg / sqrt(M_PI);
	param.a = 3.0 / param.k + 1.0;

	int arrDim = (param.vco / param.inc + 1);
	
	param.arrCreate(arrDim);

	populate(param);

	cout << param.dataPower[4] << endl;
	cin.get();

	output(param, arrDim);
}

void output(parameters &param, int arrDim)
{
	cout << "Index\tProbability\tPower\n";
	for (int i = 0; i <= arrDim; i++)
	{
		cout << i << '\t' << param.dataPDF[i] << '\t' << param.dataPower[i] << endl;
	}

	cin.get();
}


Thank you, in advance!
closed account (o3hC5Di1)
Hi there,

Consider this function:

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
 
void populate(parameters &param)
{
	int index = 0;

	for (double V = 0; V <= param.vco; V += param.inc)
	{
		if (V < param.vci)
		{
			param.dataPower[index] = 0;
			param.dataPDF[index] = fV(param, V);
		}
		else
		{
			param.dataPower[index] = pGen(param, V);
			param.dataPDF[index] = fV(param, V);
		}
	}
}


You are using "index" as an index number to your arrays. You initialize it to zero, but then afterwards you don't increment it. Accordingly, array[0] will be accessed time and time again and overwritten. You will also want to put a check on whether the index >= arraysize, because if it does, you will be accessing memory outside of the array which will lead to undefined behaviour.

Hope that helps.

All the best,
NwN
fatirishman53 (34)
You have got to be kidding me! Well... it's confirmed... I'm an idiot. Thank you so much for your help!

Since the code is already here, if anyone has any other suggestions, I am more than open to them.
Last edited on
closed account (o3hC5Di1)
Hi there,

No worries - we've all done similar things (well, I know I have anyway, I shouldn't generalize).

Perhaps one suggestion: In your constructor, initialize your pointers to nullptr or NULL. If you don't and you try to access them before you call populate(). again you can get into undefined behaviour and hard-to-track segmentation faults. Accessing a nullptr will cause segmentation faults too, but at least you can figure them out more easily in a debugger.

To do so, might I suggest using an initializer list:

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
 
parameters() : dataPower(nullptr), dataPDF(nullptr)
	{
		k = 0;
		c = 0;
		a = 0;
		vci = 0;		// Cut-in velocity
		vr = 0;		// Rated velocity
		vco = 0;		// Cut-out velocity
		vavg = 0;	// Average velocity
		area = 0;	// Turbine blade area
		airD = 0;	// Air density
		pr = 0;		// Rated power
		inc = 0;		// Velocity iteration increment
		CF = 0;		// Capacity factor
		pavg = 0;	// Average power
	}


All the best,
NwN
fatirishman53 (34)
Thanks for the tip! I've been dabbling with C++ for about a year, so I'm still, very much, a novice. I always appreciate tips on how to create more efficient and well structured code.
closed account (o3hC5Di1)
Hi there,

As we try to promote the new C++11 features here, perhaps another suggestion could be not to use raw pointers. Either use a smart pointer such as std::unique_ptr<> or in your case (where they serve as containers), use std::vector or std::array. They handle all the underlying memory-pitfalls for you. Information on all of these is available in the "reference" section of this website.

I've been using C++ for about a year now too, being on this forum has really helped to expand my knowledge, so make sure to stick around :)

All the best,
NwN
Last edited on
fatirishman53 (34)
I will be sure to research those items!

I started out using the arrays to get the program going, with the intentions of having a 2D dynamic array for the final data (instead of 2 or 3 1D arrays). I was having a hard enough time figuring out how to make a 1D dynamic array class member. Perhaps one of your proposed items would serve my purpose more efficiently.

I certainly intend on sticking around. Thanks again!!
MikeyBoy (5631)
#include "parameters.cpp"

This is setting off alarm bells. Why are you including a .cpp file in other files like this? That's a very unusual thing to do.
TheIdeasMan (6781)
Just further to the other advice:

It is not a good idea to make your member variables public. Make them all private, and then provide an interface of public functions to interact with them. Don't be tempted to provide get / set functions for each one though.

Also, it is normal practice to put the definition of your member functions into their own .cpp file. Include the header file for any .cpp file that needs to use that class. As MikeyBoy says, it is bad to include .cpp files, the compiler will find all the function definitions as long as you include the header. There is a bit of confusion there - your Functions.h looks as though it should be a Functions.cpp file. I am saying add the constructor, arrCreate and destructor to it as well. Here is an example, using the scope operator:

1
2
3
4
5
6
7
8
 
double parameters::CF()
{
	return pow(vr, -3.0) * 
			incGamma(a, vr) - 
			incGamma(a, vci) + 
			exp( - pow(vco / c, k) ) - 
			exp( - pow(vr / c, k) );
}


This brings me to another point: All the functions you have take a reference to a parameters object, which is what makes me think they should be class functions (which is why I said about the Functions.h file) - at least in a simple implementation. If this is so, then you can refer to your member variables directly (I edited your code), and there is then no need to have them public. It just seems messy to have a load of external functions that deal exclusively with member variables.

A more complicated implementation might be like what I described here:

http://www.cplusplus.com/forum/lounge/111525/


No one has commented one way or the other on this (does that mean it's right?). I wonder if I could indulge MikeyBoy or anyone else to comment on how this might work for the OP, or reply to my topic?

Also you should have a constructor that takes arguments for all your member variables, have it use an initialiser list like NwN suggested - but for all the member variables. Then you don't have to initialise them all in main().

With your variable names, don't abbreviate them too much - I would prefer names such as VelocityCutOut rather than vco, and TurbineBladeArea rather than area. This way the code can be self documenting just on the variable & function names. With member variables it is a handy and common to name them with a leading m_.

With comments, you can use them to:
1. Describe what the class is for
2. 2 or 3 lines to describe functions do, or copy & paste in a formula
3. preconditions & postconditions like range of acceptable values
4. Any other reference material like a website URL that might be of benefit

In summary, you want to make the code as clear as possible for anyone who has to read it - including yourself in 2 years time say.

Hope all is well at your end :+)
closed account (o3hC5Di1)
TheIdeasMan wrote:
It is not a good idea to make your member variables public. Make them all private, and then provide an interface of public functions to interact with them. Don't be tempted to provide get / set functions for each one though.


TheIdeasMan wrote:
This brings me to another point: All the functions you have take a reference to a parameters object, which is what makes me think they should be class functions (which is why I said about the Functions.h file) - at least in a simple implementation. If this is so, then you can refer to your member variables directly (I edited your code), and there is then no need to have them public. It just seems messy to have a load of external functions that deal exclusively with member variables.


Both of these statements are the topic of debate within the OOP community. As for accessors/mutators I tried getting some opinions from some of the experts around here and I'm still not entirely clear on it. In particular: it's often said that if you design your class well, you may have some public members and some for which you provide accessors/mutators. It's kind of hard as a beginner to decide on which one to use though, because you may want to alter just about anything in a class at some point because you realize you were mistaken, so experience seems to play a role here too. Some topics topic I found interesting in this regard:
http://www.cplusplus.com/forum/beginner/6415/
http://www.cplusplus.com/forum/beginner/88994/
This interview with Bjarne Stroustrup also deals with the subject: http://www.artima.com/intv/goldilocks3.html.

As for member functions vs. free functions, Scott Meyers makes a pretty compelling argument in favour of free functions over here: http://www.drdobbs.com/cpp/how-non-member-functions-improve-encapsu/184401197 . In the same article, he makes a case for accessors/mutators, so as I said: controversial.


I'm not trying to break down your statements, by the way, just wanted to mention that opinions on these matters seem to defer quite a bit.

All the best,
NwN
Last edited on
TheIdeasMan (6781)
Hi NwN,

A while back I started this topic:

http://www.cplusplus.com/forum/lounge/101305/


and it turned into a 7 page behemoth !! With regard to trivial set functions, It is one of those topics that falls into the category (IMO) of "Don't do it, but it's OK if you know what you are doing, or it's OK sometimes", but perhaps not in the same league as goto, infinite loops etc.

So there are cases where these are OK. But beginners shouldn't routinely do it.

It seems to me that public member variables are always a bad idea, as are set functions which are straight assignment. There is often no need to provide a get / set function for each member variable. Get functions are pretty benign & increase encapsulation. Update functions like Scale, Rotate, Move, Stretch in a CAD system say, are a different thing to set functions and should include checking and validation.

Also when dealing with beginners, it seems best to advise them not to do this, because they so easily fall into abusing them. Beginners seem to gravitate to taking the path of least resistance, so they have get /set functions rather than learn about constructors with initialiser lists, and the idea that member functions have access to member variables.

With the external functions, Scott Meyer's article about them (and some other events) prompted me to think of what I said in my topic:

http://www.cplusplus.com/forum/lounge/111525/


Amongst other things, I was proposing to collect all the external functions into their own friend class, thereby keeping the Data class as just data. I would be interested to hear what do you (and others) think? Maybe not here (so as not to derail this topic), but reply to my topic if you would.

Could I troll for comment by asking if I can have the NERVE to call my proposal a Design Pattern?

I think it is a shame that Meyers used a point class in his article - it seems to reinforce existing confusion: Why aren't the members x and y public then? I think he mentioned it to demonstrate the benefits of having an interface. But beginners see this trivial example (not thinking deeply about more complex usage he mentioned) in his article and in countless textbooks by others, and think it is a good idea, then use it to implement their bank account assignments! Code::Blocks even has a "Add new Getter & Setter" options for member variables in it's class wizard.

Going back to the OP's problem, I suggested making those functions part of the class as a simple & tidy implementation, which is better than what he has now. Obviously there are technically better solutions, but I was trying to keep it simple. He does need to separate his Data Container from the class though IMO.

NwN wrote:
I'm not trying to break down your statements, ....


No worries, it is obvious you are a nice person, so I wasn't reading anything negative into what you said. Also, more discussion about these debatable topics is better for everyone's education, I reckon.

Cheers :+)

Last edited on
closed account (o3hC5Di1)
Hey,

Thanks very much for linking me to your 7-page topic, I'm going to read through it now.
I would suggest perhaps altering your topic about the friend class, when I read the post I was a little confused as to what you meant exactly - perhaps include a code example.

I was proposing to collect all the external functions into their own friend class


I think that's logical thinking, but it kind of seems to break encapsulation. If these functions need access to any private members, they should probably be member functions. If they can get by just using the public interface, they could be free functions - I believe that's more or less what Scott Meyers is trying to say anyway. The point of abstractions is that they provide an interface, but hide the implementation from the user. Making these functions friend functions would expose the implementation. Also, one of Meyer's main points is that having free functions makes a more uniform interface if you need to extend the interface yourself. You wouldn't be able to add a free friend function to a class any more than you would be able to add a member function as far as I can see. Anyway - just my nooby 2 cents.

Recently I saw a topic coming by where someone tried to emulate C#-type accessors, which allow you to do data.x, but in practise call the actual getter/setter. I think for C++ it seems superfluous and perhaps a little dangerous, because most people are not used to it.

Thanks again for linking to those topics.

All the best,
NwN
TheIdeasMan (6781)
I am going to answer here:

http://www.cplusplus.com/forum/general/111525/#msg614296


Because this is no longer part of the OP's topic.
fatirishman53 (34)
Wow! I figured I would have gotten e-mail notifications of further replies to this...

Thank you everyone for a ton of additional information to look over and play with! I certainly appreciate your pointers and suggestions!

If it matters:
About the whole class members issue, I hadn't really decided how I wanted to structure those yet. If I had a user interface setup, they would all be either input by the user or calculated from user inputs. Creating a class made it easier to keep track of the variables while developing the equations and the flow of calculations. Then, I just slapped some numbers in to verify calculation results. You all have certainly given me more to think about, though!

If anyone would like, I can re-post the revised code.
fatirishman53 (34)
This link was broken:
This interview with Bjarne Stroustrup also deals with the subject: http://www.artima.com/intv/goldilocks3.html


Do you have another reference to this NwN?
closed account (o3hC5Di1)
Hi there,

The link works for me?

All the best,
NwN
MikeyBoy (5631)
Wow! I figured I would have gotten e-mail notifications of further replies to this...

You can, but you have to go to the drop-down at the bottom of the thread called "Tracking options for this topic", and select "Subscribe". I wish it would do it automatically when you post to a thread.
TheIdeasMan (6781)
fatirishman53 wrote:
If anyone would like, I can re-post the revised code.


Yes that would be interesting: to see how it matches up with what us respondents had collectively envisaged.

Also, I think it is great that you have taken advice on board and altered your code, rather than the common attitude: "It works now, so I will leave it". Cheers :+)
fatirishman53 (34)
@NwN
BAH!... It's working now! It was giving me an error before :)

@MikeyBoy
Thanks! I guess I shouldn't have assumed that it would automatically subscribe me.

@TheIdeasMan
I will post them up as soon as I implement some more of the ideas you all suggested. This code could possibly evolve into something I use for my thesis, so I figured I would take the opportunity to learn more about structure, efficiency, and C++ in general. Beside that, it's good practice!
TheIdeasMan (6781)
@NwN

Thanks for that link, it is working for me now too.

@everyone

All 4 pages were worth reading, plus some of the other interviews as well. I think it is great that Stroustrup talks about things at a beginners level in this interview.


There was this on page 2:

Stroustrup wrote:
...... I particularly dislike classes with a lot of get and set functions. That is often an indication that it shouldn't have been a class in the first place. It's just a data structure. And if it really is a data structure, make it a data structure.


With the invariants & math formulae, it seems to me that a formula needs to go in a class rather than a public struct, because one can't change 1 variable by it self without invalidating all the rest, so the member functions should be responsible for maintaining the object in a valid state at all times.

@OP

I was going to have a go at re-writing your code - so we could compare once you have done yours. Just wondering what pre-conditions might exist for the data? That is: valid minimum & maximum ranges; acceptable precision values; or anything else that might lead to invalid data. Perhaps a wiki page for the formulae you are using, then I could have a go at working out some that stuff myself. Although if this is your area of expertise, then it is better to get it from the horse's mouth (so to speak :+D )
Pages: 12