Thread: something wrong with the idea of creating a struct - how to fix it?

  1. #1
    Registered User
    Join Date
    Dec 2009
    Posts
    32

    something wrong with the idea of creating a struct - how to fix it?

    Hi,

    Below is the first exercise on struct that I did. I have a feeling that I didn't get an idea behind the struct. Looking at the program I wrote, I am wondering what is the benefit of writing this program using struct. I could have write 5 functions for mean, median, smallest, largest, and call each of them when printing the results. The exercise asks me to create a struct, so it must imply that there's some benefit from using struct. I'm suspecting that I didn't get the idea... Could you please suggest some improvement? Thanks a lot...

    Code:
    // Exercise 8.12
    #include "std_lib_facilities.h"
    /*
    	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?
    
    */
    
    struct stats {
    	vector<double> v;
    	double smallest, largest, median;
    	double fmean(vector<double>& v);
    	double fmedian(vector<double>& v);
    	double fsmallest(vector<double>& v);
    	double flargest(vector<double>& v);
    };
    
    int main()
    try {
    	stats mystats;
    	vector<double> v;
    	double x;
    	cout << "Enter some elements for a vector: \n";
    	while (cin >> x)
    		v.push_back(x);
    	cout << "Here are some summary statistics: \n";
    	cout << "Smallest: " << mystats.fsmallest(v) << '\n';
    	cout << "Largest: " << mystats.flargest(v) << '\n';
    	cout << "Mean: " << mystats.fmean(v) << '\n';
    	cout << "Median: " << mystats.fmedian(v) << '\n';
    	
    }
    catch (runtime_error e) {
    	cout << e.what();
    }
    
    double stats::fsmallest(vector<double>& v)
    {
    	smallest = v[0];
    	for (unsigned int i = 0; i < v.size(); ++i) {
    		if (v[i] < smallest)
    			smallest = v[i];
    	}
    	return smallest;
    }
    
    double stats::flargest(vector<double>& v)
    {
    	largest = v[0];
    	for (unsigned int i = 0; i < v.size(); ++i) {
    		if (v[i] > largest)
    			largest = v[i];
    	}
    	return largest;
    }
    
    double stats::fmean(vector<double>& v)
    {
    	double sum = 0;
    	for (unsigned int i = 0; i < v.size(); ++i) {
    		sum += v[i];
    	}
    	return sum / v.size();
    }
    
    double stats:: fmedian(vector<double>& v)
    {
    	sort(v.begin(), v.end());
    	median = 0;
    	for (unsigned int i = 0; i < v.size(); ++i) {
    		if (v.size()%2==0)
    			median = (v[v.size()/2] + v[v.size()/2-1])/2;
    		else
    			median = v[v.size()/2];	
    	}
    	return median;
    }

  2. #2
    (?<!re)tired Mario F.'s Avatar
    Join Date
    May 2006
    Location
    Ireland
    Posts
    8,446
    The struct is optional as the question implies. However it is hinted you should try it.

    Meanwhile, the question doesn't tell you to do the calculations inside the struct as you are doing. For all purposes, you could create a struct that would only hold the data (min, max, mean and median, and possibly the vector). The functions could be created outside the struct and take it as a parameter. They would return void and the result of their calaculation stored in the appropriate struct data member. You would then output the struct members to the console.

    And this way you would come closer to the usual purpose of a struct in C++, which is that of data object which purpose is to only hold static data and which existence clearly indicates any logic to create and manipulate that data is somewhere else. (See POD)
    Last edited by Mario F.; 01-17-2010 at 09:11 PM.
    Originally Posted by brewbuck:
    Reimplementing a large system in another language to get a 25% performance boost is nonsense. It would be cheaper to just get a computer which is 25% faster.

  3. #3
    Registered User
    Join Date
    Dec 2009
    Posts
    32
    Thank you...
    Last edited by pantera; 01-19-2010 at 11:07 PM.

  4. #4
    Algorithm Dissector iMalc's Avatar
    Join Date
    Dec 2005
    Location
    New Zealand
    Posts
    6,318
    Don't declare vector<double> v inside the stats struct and pass vector<double> v into every member function. You either do one or the other.
    My homepage
    Advice: Take only as directed - If symptoms persist, please see your debugger

    Linus Torvalds: "But it clearly is the only right way. The fact that everybody else does it some other way only means that they are wrong"

  5. #5
    Registered User
    Join Date
    Dec 2009
    Posts
    32
    Quote Originally Posted by iMalc View Post
    Don't declare vector<double> v inside the stats struct and pass vector<double> v into every member function. You either do one or the other.
    Thank you for helping me out...

    Code:
    // Exercise 8.12
    
    #include "std_lib_facilities.h"
    
    /*
    	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?
    
    */
    
    struct stats {
    	vector<double> v;
    	double mean;
    	double median;
    };
    
    void fmean(stats& results)
    {
    	double sum = 0;
    	for (unsigned int i = 0; i < results.v.size(); ++i)
    		sum += results.v[i];
    	results.mean = sum/results.v.size();
    }
    
    void fmedian(stats& results)
    {
    	sort(results.v.begin(), results.v.end());
    	for (unsigned int i = 0; i < results.v.size(); ++i)
    		results.median = results.v.size()%2 == 0 ? (results.v[results.v.size()/2] + results.v[results.v.size()/2-1])/2 : results.v[results.v.size()/2];
    }
    
    int main()
    try {
    	stats myresults;
    	cout << "Enter some elements: \n";
    	double x;
    	while (cin >> x)
    		myresults.v.push_back(x);
    	fmedian(myresults);
    	fmean(myresults);
    	cout << "Mean: " << myresults.mean << '\n';
    	cout << "Median: " << myresults.median << '\n';
    }
    
    catch (runtime_error e) {
    	cout << e.what();
    }

  6. #6
    (?<!re)tired Mario F.'s Avatar
    Join Date
    May 2006
    Location
    Ireland
    Posts
    8,446
    Yup. That's more like what they are asking you to do. Hope that clear up a bit and allows yout to start seeing the possible advantages of using structs and classes as a means to simplify the description of data and its manipulation.

    For completeness you should observe the question requirements, which include also calculating the min and max values and store them in the struct.

    Meanwhile:

    - Your median calculation contains unnecessary code. You don't need to cycle through the vector. Just to sort and calculate. Note your for loop body is not making any use of i, neither it is accumulating the result of the calculation, which clearly indicates you don't need the loop. Just the calculation.

    - If you already went through algorithms, you should know you can simplify some calculations by taking advantage of these. For instance, double sum = accumulate(results.v.begin(), results.v.end(), 0); will return the sum of all values in the vector. Check the STL Algorithms.

    EDIT:
    One small side note. It's perhaps best if you replace the ternary operator in your median calculation with the if statement. Ternary operators are nice, but when they force you to a lengthy line, loose readability.
    Last edited by Mario F.; 01-21-2010 at 05:57 AM.
    Originally Posted by brewbuck:
    Reimplementing a large system in another language to get a 25% performance boost is nonsense. It would be cheaper to just get a computer which is 25% faster.

  7. #7
    Registered User
    Join Date
    Dec 2009
    Posts
    32
    Quote Originally Posted by Mario F. View Post
    Yup. That's more like what they are asking you to do. Hope that clear up a bit and allows yout to start seeing the possible advantages of using structs and classes as a means to simplify the description of data and its manipulation.

    For completeness you should observe the question requirements, which include also calculating the min and max values and store them in the struct.

    Meanwhile:

    - Your median calculation contains unnecessary code. You don't need to cycle through the vector. Just to sort and calculate. Note your for loop body is not making any use of i, neither it is accumulating the result of the calculation, which clearly indicates you don't need the loop. Just the calculation.

    - If you already went through algorithms, you should know you can simplify some calculations by taking advantage of these. For instance, double sum = accumulate(results.v.begin(), results.v.end(), 0); will return the sum of all values in the vector. Check the STL Algorithms.

    EDIT:
    One small side note. It's perhaps best if you replace the ternary operator in your median calculation with the if statement. Ternary operators are nice, but when they force you to a lengthy line, loose readability.
    Thank you so much for helping me... Below is the revised program I wrote based on your suggestion...

    Code:
    // Exercise 8.12
    
    #include "std_lib_facilities.h"
    #include <numeric>
    
    /*
    	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?
    
    */
    
    struct stats {
    	vector<double> v;
    	double mean;
    	double median;
    	double max;
    	double min;
    };
    
    void fmean(stats& results)
    {
    	double sum = accumulate(results.v.begin(), results.v.end(), 0);
    	results.mean = sum/results.v.size();
    }
    
    void fmedian(stats& results)
    {
    	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];
    }
    
    void fmin_max(stats& results)
    {
    	results.max = results.min = results.v[0];
    	for (unsigned int i = 0; i < results.v.size(); ++i) {
    		if (results.v[i] > results.max)
    			results.max = results.v[i];
    		if (results.v[i] < results.min)
    			results.min = results.v[i];
    	}		
    			
    }
    
    int main()
    try {
    	stats myresults;
    	cout << "Enter some elements: \n";
    	double x;
    	while (cin >> x)
    		myresults.v.push_back(x);
    	fmedian(myresults);
    	fmean(myresults);
    	fmin_max(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();
    }

  8. #8
    (?<!re)tired Mario F.'s Avatar
    Join Date
    May 2006
    Location
    Ireland
    Posts
    8,446
    Think of ways to improve your fmin_max function. Think of what happens when you sort the vector, for instance. Or explore the link to the STL algorithms I gave you earlier and see if there is anything useful in there.
    Originally Posted by brewbuck:
    Reimplementing a large system in another language to get a 25% performance boost is nonsense. It would be cheaper to just get a computer which is 25% faster.

  9. #9
    Registered User
    Join Date
    Dec 2009
    Posts
    32
    Quote Originally Posted by Mario F. View Post
    Think of ways to improve your fmin_max function. Think of what happens when you sort the vector, for instance. Or explore the link to the STL algorithms I gave you earlier and see if there is anything useful in there.
    Thank you. I've revised the program based on your hint.

    - For min/max, I just used first element for min & last element for max, since it's been sorted, i.e. results.min = results.v[0]; results.max = results.v[results.v.size()-1]; In the STL, I've found max_element, and min_element for the same purpose...

    - I used accumulate from STL to calculate the sum;

    Thanks a lot for your time.

Popular pages Recent additions subscribe to a feed

Similar Threads

  1. Link List math
    By t014y in forum C Programming
    Replies: 17
    Last Post: 02-20-2009, 06:55 PM
  2. Replies: 1
    Last Post: 12-03-2008, 03:10 AM
  3. Looking for constructive criticism
    By wd_kendrick in forum C Programming
    Replies: 16
    Last Post: 05-28-2008, 09:42 AM
  4. Dikumud
    By maxorator in forum C++ Programming
    Replies: 1
    Last Post: 10-01-2005, 06:39 AM
  5. problem with structures and linked list
    By Gkitty in forum C Programming
    Replies: 6
    Last Post: 12-12-2002, 06:40 PM