patterncppModerate
Testing parity of number of items
Viewed 0 times
itemsnumbertestingparity
Problem
This is my first C++ program with classes, and I don't want to develop bad skills. It's very simple and consists of 3 files. This is an exercise from a book, hence the name of the driver file.
Please tell me whether it's proper or not!
parity.cpp:
parity.h:
13_1.cpp
Please tell me whether it's proper or not!
parity.cpp:
#include
#include "parity.h"
void Parity::put(int number)
{
numbers.push_back(number);
}
bool Parity::test(void)
{
if (numbers.size() % 2 == 0)
return true;
else return false;
}parity.h:
#ifndef PARITY_H_
#define PATITY_H_
#include
#include
class Parity
{
private:
std::vector numbers;
public:
void put(int number);
bool test(void);
};
#endif13_1.cpp
#include
#include "parity.h"
int main()
{
int number;
bool odd_number;
Parity parity_test;
while(true)
{
std::cout > number;
if (number == 0)
{
break;
}
else
{
parity_test.put(number);
odd_number = parity_test.test();
std::cout << odd_number;
}
}
return 0;
}Solution
Some general guidelines regarding
-
You have a typo in your header guard:
-
The class isn't doing anything with `
#includes:- only
#includein the header when necessary (this reduces potentially unwanted dependencies)
- whenever possible, forward declare in the header instead of using
#include
- whenever possible,
#includein the implementation only (this does not affect dependency)
- never
#includea source file (having to do so indicates a hierarchy issue)
- be sure to never allow files to
#includeeach other (sort of in a "circular" manner)
-
You have a typo in your header guard:
#ifndef PARITY_H_
#define PATITY_H_ // should be PARITY_H_-
The class isn't doing anything with `
and , so just remove them.
-
test() doesn't need the void parameter; that's only for C.
-
This:
if (numbers.size() % 2 == 0)
return true;
else return false;
can be shortened to this:
return (numbers.size() % 2 == 0); // the statement's Boolean value is returned
-
You can use std::boolalpha to display a bool as "true" for 1 or "false" for 0:
std::cout << std::boolalpha << odd_number;
-
The while and if-else don't seem too intuitive. The loop just runs until you break from it, and it's not directly tied to number`. One alternative is to do an initial input before the loop:// initial input before loop-- number may be 0
std::cout > number;
// if not 0, proceed until it's 0
while (number != 0)
{
parity_test.put(number);
bool odd_number = parity_test.test(); // initialize here instead
std::cout > number;
}Code Snippets
#ifndef PARITY_H_
#define PATITY_H_ // should be PARITY_H_if (numbers.size() % 2 == 0)
return true;
else return false;return (numbers.size() % 2 == 0); // the statement's Boolean value is returnedstd::cout << std::boolalpha << odd_number;// initial input before loop-- number may be 0
std::cout << "Enter an integer: ";
int number; // move the declaration here
std::cin >> number;
// if not 0, proceed until it's 0
while (number != 0)
{
parity_test.put(number);
bool odd_number = parity_test.test(); // initialize here instead
std::cout << "odd number? " << std::boolalpha << odd_number;
// input again
std::cout << "Enter an integer: ";
std::cin >> number;
}Context
StackExchange Code Review Q#30779, answer score: 10
Revisions (0)
No revisions yet.