What is wrong with this C++ program?

Discussion in 'Mac Programming' started by pantera, Jan 23, 2010.

  1. pantera macrumors newbie

    Joined:
    Jan 23, 2010
    #1
    Hi,

    I still can't figure out where this program goes wrong. It keeps producing undefined behavior... Please help. Thank you.

    Code:
    // Exercise 8.12 - Version 1
    
    #include "std_lib_facilities.h"
    
    /*
    	[COLOR="Red"]Write a function that finds the smallest & the largest element of a vector
    	argument and also compute the mean and the median. Do not use global variables.
    	Either return a struct containing results or pass them back through reference
    	arguments. Which of the 2 ways of returning several result values do you prefer and why?[/COLOR]
    
    */
    
    struct Stats_results {
    	double mean;
    	double median;
    	double max;
    	double min;
    };
    
    Stats_results compute_stats(vector<double> data)
    {
    	Stats_results results;
    	
    	// mean
    	double sum = 0;
    	for (unsigned int i = 0; i < data.size(); ++i)
    		sum += data[i];
    	results.mean = sum / data.size();
    	
    	// median
    	sort(data.begin(), data.end());
    	if (data.size()%2==0) 
    		results.median = (data[data.size()/2] + data[data.size()/2 - 1])/2;
    	else
    		results.median = data[data.size()/2];
    		
    	// max
    	results.max = data[data.size() - 1];
    	
    	// min
    	results.min = data[0];
    	
    	return results;
    }
    
    int main()
    try {
    	Stats_results myresults;
    	cout << "Enter some numbers for a vector: \n";
    	vector<double> v;
    	double x;
    	while (cin >> x)
    		v.push_back(x);
    	
    	if (v.size() == 0) error("Empty vector!\n"); // make sure the user input data	
    	
    	compute_stats(v);
    	cout << "Mean: " << myresults.mean << '\n';
    	cout << "Median: " << myresults.median << '\n';
    	cout << "Max: " << myresults.max << '\n';
    	cout << "Min: " << myresults.min << '\n';
    }
    
    catch (runtime_error e) {
    	cout << e.what() << '\n';
    }
     
  2. lee1210 macrumors 68040

    lee1210

    Joined:
    Jan 10, 2005
    Location:
    Dallas, TX
    #2
    Where do you think myresults is being set to the return value of compute_stats?

    I think you just found a reason not to use this method, and push you toward passing some reference arguments.

    -Lee

    P.S. You said you're teaching yourself, so this isn't like traditional HW, but you should mention if you don't want people to just solve your problem and post the code. You will learn less if they do.
     
  3. chown33 macrumors 604

    Joined:
    Aug 9, 2009
    #3
    I'm no C++ expert, but it looks like myresults in main() is never assigned a value. Specifically, not the return value from compute_stats(). Therefore, it's an uninitialized local variable whose value is being used incorrectly.
     
  4. pantera thread starter macrumors newbie

    Joined:
    Jan 23, 2010
    #4
    Thank you for the hint. This is an exercise from the book "Programming: Principles and Practice using C++". I have done this exercise for one version and it produces the correct results. Then, I tried to modify it a bit using different functions (I'm trying to come up with different ways to get to the solution). I will try to look at it again & see how I can fix it. Thank you for your time.

    My previous version is like below:

    Code:
    // Exercise 8.12 - Version 4
    
    
    #include "std_lib_facilities.h"
    
    [COLOR="Red"]/*
    	Write a function that finds the smallest & the largest element of a vector
    	argument and also compute the mean and the median. Do not use global variables.
    	Either return a struct containing results or pass them back through reference
    	arguments. Which of the 2 ways of returning several result values do you prefer and why?
    
    */[/COLOR]
    
    struct stats {
    	vector<double> v;
    	double mean;
    	double median;
    	double max;
    	double min;
    };
    
    void fstats(stats& results)
    {
    	// mean
    	double sum = 0;
    	for (unsigned int i = 0; i < results.v.size(); ++i)
    		sum += results.v[i];
    	results.mean = sum / results.v.size();	
    	
    	// median
    	sort(results.v.begin(), results.v.end());
    	if (results.v.size()%2 == 0) 
    		results.median = (results.v[results.v.size()/2] + results.v[results.v.size()/2-1])/2;
    	else
    		results.median = results.v[results.v.size()/2];
    	
    	// max & min
    	results.min = results.v[0];
    	results.max = results.v[results.v.size()-1];
    		
    }
    
    int main()
    try {
    	stats myresults;
    	cout << "Enter some elements: \n";
    	double x;
    	while (cin >> x)
    		myresults.v.push_back(x);
    	fstats(myresults);
    	cout << "Mean: " << myresults.mean << '\n';
    	cout << "Median: " << myresults.median << '\n';
    	cout << "Max: " << myresults.max << '\n';
    	cout << "Min: " << myresults.min << '\n';
    }
    
    catch (runtime_error e) {
    	cout << e.what();
    }
     
  5. pantera thread starter macrumors newbie

    Joined:
    Jan 23, 2010
    #5
    I've been finding learning C++ really hard. So I need a lot of help from online forums. I don't have the opportunity to attend a class, nor do I have a friend who is a CS that I can turn to for help when need arises. I'm learning it in my free time (whenever I have free time, even during X-mas/New Year time...). Reading the chapter and understanding the idea behind many difficult concepts takes time, and this is not enough. Doing exercises at the end of chapters and reading other people's codes should be essential, practicing writing codes, etc... All these things in reality takes so much time, so I've been going much slower than a normal uni C++ intro course (they've covered the whole book of 20 chapters in 14 weeks). I've spent 14 weeks already, but still at the Chapter 10/11)... Thank you for taking time to reply and giving the hint...
     
  6. pantera thread starter macrumors newbie

    Joined:
    Jan 23, 2010
    #6
    Thank you, chown33, for the hint...
     
  7. Sander macrumors 6502

    Joined:
    Apr 24, 2008
    #7
    Your previous version worked because you passed the "stats" structure to your function by reference, with the function "filling it in" for you.

    When you modify the code to make the function return the stats, you must also make sure to change the calling site and assign this function result to some local variable for further use.

    One small idiomatic thing: You pass the vector "by value" to your function, which means a copy is made. For small data sets this isn't a problem, but for a vector containing a gazillion entries, it is. That's why experienced C++ programmers usually pass these by const reference.

    Now, in your case, you actually modify the vector (because you're sorting it), and modifying the caller's data inside the callee is usually considered "impolite" (if it was a const reference, it wouldn't even compile). Since you'd have to copy the data anyway with your implementation, you might as well do so during the call, so your code is fine. However, since passing a vector by value is the exception, you might want to add a line of comment explaining why you're doing it.
     

Share This Page