patterncppMinor
Compute and print the sum and mean of name and value pairs
Viewed 0 times
thecomputemeanvalueprintnamesumandpairs
Problem
I'm just looking for more feedback on style/approach regarding this small problem. Also, if you have tips to make it more readable or C++-like, please let me know.
Also, why is the problem asking for the sum and mean of each name and all names, while the sum of a single element is itself? Am I missing anything in terms of answering the question?
Also, why is the problem asking for the sum and mean of each name and all names, while the sum of a single element is itself? Am I missing anything in terms of answering the question?
#include
#include
#include
#include
#include
#include
#include
#include
/********************************************************************
* Read a sequence of possibly whitespace-separated (name,value)
* pairs, where the name is a single whitespace-separated word and
* the value is an integer or floating-point value. Compute and print
* the sum and mean for each name and the sum and mean for all names
* ******************************************************************/
using namespace std;
std::multimap _map;
istream& operator>>(istream& stream, pair &in ) {
return stream >> in.first >> in.second;
}
ostream& operator &out ) {
return stream input;
(*is) >> input;
_map.insert(input);
} while( is->peek( ) != EOF );
ostream *os = &cout;
multimap::iterator mit = _map.begin( );
float sum = 0.0;
while( mit != _map.end( ) ) {
pair p_pair = (*mit);
(*os) ( sum/_map.size( ) );
(*os) << "Sum: " << sum << " Mean: " << mean << endl;
}Solution
Although it might be a bit longer (I haven't bothered to count lines), I think I'd prefer something on this order:
If you preferred, you could use an
While I suppose others might disagree, I prefer this for the simple reason that I find it easier to understand. We start by reading in all the data. We then find each run of identical keys, and compute the average for that run and print it out. When we reach the end of the data, we do the same for the overall average.
In fairness, if you were working with huge amounts of data, this probably wouldn't be the best way do do things. It does spend more time walking through the same data than is strictly necessary. At the same time, unless you're going to rewrite the code to overlap reading the data with computing the result, it's unlikely to make much real difference -- almost regardless of how you do the computation, that I/O will almost certainly occupy the vast majority of the overall time.
#include
#include
#include
#include
#include
#include
#include
typedef std::multimap vtype;
typedef std::pair ptype;
namespace std {
std::istream &operator>>(std::istream &is, ptype &v) {
return is >> v.first >> v.second;
}
}
float add_second(float a, vtype::value_type b) {
return a + b.second;
}
template
float average(FwdIt start, FwdIt end) {
float total = std::accumulate(start, end, 0.0f, add_second);
return total / std::distance(start, end);
}
int main() {
vtype values((std::istream_iterator(std::cin)),
std::istream_iterator());
vtype::iterator start, end;
for (start = values.begin(); start != values.end(); start=end) {
end = values.upper_bound(start->first);
std::cout first << ": " << average(start, end) << "\n";
}
std::cout << "Overall: " << average(values.begin(), values.end()) << "\n";
return 0;
}If you preferred, you could use an
std::vector instead of an std::multimap. That would require some minor changes to the code (e.g., using std::upper_bound instead of std::multimap::upper_bound, and adding a call to std::sort after reading the data, but would otherwise be pretty similar. It would, however, probably be a bit more efficient (less CPU time and memory).While I suppose others might disagree, I prefer this for the simple reason that I find it easier to understand. We start by reading in all the data. We then find each run of identical keys, and compute the average for that run and print it out. When we reach the end of the data, we do the same for the overall average.
In fairness, if you were working with huge amounts of data, this probably wouldn't be the best way do do things. It does spend more time walking through the same data than is strictly necessary. At the same time, unless you're going to rewrite the code to overlap reading the data with computing the result, it's unlikely to make much real difference -- almost regardless of how you do the computation, that I/O will almost certainly occupy the vast majority of the overall time.
Code Snippets
#include <map>
#include <string>
#include <numeric>
#include <iostream>
#include <utility>
#include <iterator>
#include <algorithm>
typedef std::multimap<std::string, float> vtype;
typedef std::pair<std::string, float> ptype;
namespace std {
std::istream &operator>>(std::istream &is, ptype &v) {
return is >> v.first >> v.second;
}
}
float add_second(float a, vtype::value_type b) {
return a + b.second;
}
template <class FwdIt>
float average(FwdIt start, FwdIt end) {
float total = std::accumulate(start, end, 0.0f, add_second);
return total / std::distance(start, end);
}
int main() {
vtype values((std::istream_iterator<ptype>(std::cin)),
std::istream_iterator<ptype>());
vtype::iterator start, end;
for (start = values.begin(); start != values.end(); start=end) {
end = values.upper_bound(start->first);
std::cout << start->first << ": " << average(start, end) << "\n";
}
std::cout << "Overall: " << average(values.begin(), values.end()) << "\n";
return 0;
}Context
StackExchange Code Review Q#3783, answer score: 3
Revisions (0)
No revisions yet.