HiveBrain v1.2.0
Get Started
← Back to all entries
patterncppModerate

Testing parity of number of items

Submitted by: @import:stackexchange-codereview··
0
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:

#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);
};

#endif


13_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 #includes:

  • only #include in the header when necessary (this reduces potentially unwanted dependencies)



  • whenever possible, forward declare in the header instead of using #include



  • whenever possible, #include in the implementation only (this does not affect dependency)



  • never #include a source file (having to do so indicates a hierarchy issue)



  • be sure to never allow files to #include each 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 returned
std::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.