\$\begingroup\$
\$\endgroup\$
2
I had to write a program that accepted five numbers from the user. Then it will display the number of odd and even numbers, and a total sum of all numbers.
#include <iostream>
using namespace std;
int main()
{
int a;
int b;
int c;
int d;
int e;
int sum;
int remainder;
int bremainder;
int cremainder;
int dremainder;
int eremainder;
int even;
int odd;
int total = 0;
int Ototal = 0;
cout << "Please enter the 5 numbers you would like calculated: \n";
cin >> a;
cin >> b;
cin >> c;
cin >> d;
cin >> e;
remainder = a % 2;
bremainder = b % 2;
cremainder = c % 2;
dremainder = d % 2;
eremainder = e % 2;
sum = (a + b + c + d + e);
if (remainder = 0)
{
total+=remainder;
}
else {
Ototal += remainder;
}
if (bremainder = 0)
{
total += bremainder;
}
else {
Ototal += bremainder;
}
if (cremainder = 0)
{
total += cremainder;
}
else {
Ototal += cremainder;
}
if (dremainder = 0)
{
total += dremainder;
}
else {
Ototal += dremainder;
}
if (eremainder = 0)
{
total += eremainder;
}
else {
Ototal += eremainder;
}
cout << total << "\n";
cout << Ototal << "\n";
cout << sum << "\n";
system("PAUSE");
return 0;
}
Jamal
35.2k13 gold badges134 silver badges238 bronze badges
asked Oct 13, 2014 at 6:03
user55918user55918
2 Answers 2
\$\begingroup\$
\$\endgroup\$
2
- Do not use
system("pause")
. Read this - Stop using
using namespace std
. Read this - You have an unneccessary amount of variables. You don't need to have a variable to hold the remainder for each number.
- Just have a function that determines whether the input number is odd or even. I think you might have to rethink the logic of your program.
//Your function should do the following:
//MAX is a const int. In your case it is 5
for(int i=0; i<MAX; i++){
if(numbers[i] % 2 == 0)
evenNumbers++;
else
oddNumbers++;
sum += numbers[i];
}
Here is how I would have done it:
#include <iostream>
#include <vector>
#include <iterator>
using std::cin;
using std::cout;
using std::endl;
using std::vector;
int main(){
int value = 0;
int oddNumbers(0), evenNumbers(0), sum(0);
std::istream_iterator<int> begin(cin);
std::istream_iterator<int> end;
//while(cin >> value)
// numbers.push_back(value);
//Instead of a while loop, I use input stream iterators.
vector<int> numbers;
std::back_insert_iterator<std::vector<int>> iter (numbers);
std::copy(begin,end,iter);
//Range based for-loop (C++11)
for(auto i : numbers){
if(i % 2 == 0){
evenNumbers++;
}else {
oddNumbers++;
}
sum += i;
}
cout << "# of odd numbers: " << oddNumbers << "\n";
cout << "# of even numbers: " << evenNumbers << "\n";
cout << "Sum of all numbers: " << sum << endl;
}
answered Oct 13, 2014 at 6:06
-
3
-
\$\begingroup\$ @elmo But why? The article you linked explicitly says that 'always using curly braces' blindly does not prevent errors... \$\endgroup\$CiaPan– CiaPan2017年12月06日 16:27:52 +00:00Commented Dec 6, 2017 at 16:27
\$\begingroup\$
\$\endgroup\$
Inspired by @EngieOPs answer, I ended up with the following:
- I use
std::copy_n
instead ofstd::copy
to read the values, because it was asking for a fixed sized input. - I templated everything to enable counting of other types than
int
. - I created a functor
is_even
for better readability. - I use
std::count_if
and accumulate from the STL to calculate the end results. - Instead of counting the odd numbers, I just use the difference between the number of inputs and the number of even variables.
I think the usage of std::count_if
and std::accumulate
is much more readable than a for
loop. Whether a template makes sense would depend on the actual usage.
#include <iostream>
#include <vector>
#include <iterator>
#include <algorithm>
#include <numeric>
struct is_even
{
template <class T>
bool operator() (T value)
{
return (value % 2) == 0;
}
};
template<class T>
void processInput(std::size_t count)
{
std::istream_iterator<T> begin(std::cin);
std::vector<T> numbers;
std::back_insert_iterator<std::vector<T>> iter(numbers);
std::copy_n(begin, count, iter);
auto evenNumbers = std::count_if(numbers.begin(), numbers.end(), is_even());
auto oddNumbers = numbers.size() - evenNumbers;
auto sum = std::accumulate(numbers.begin(), numbers.end(), T());
std::cout << "# of odd numbers: " << oddNumbers << "\n";
std::cout << "# of even numbers: " << evenNumbers << "\n";
std::cout << "Sum of all numbers: " << sum << std::endl;
}
int main(void)
{
processInput<int>(5);
return 0;
}
Jamal
35.2k13 gold badges134 silver badges238 bronze badges
answered Oct 13, 2014 at 9:49
lang-cpp
=
in C/C++ does NOT serve as comparision! And if it did, then after satisfying e.g. 'cremainder
equals zero' it doesn't make sense to addcremainder
tototal
, because adding zero tototal
does not change thetotal
value. \$\endgroup\$