patterncppModerate

Simple zodiac sign program

Submitted by: @import:stackexchange-codereview·Mar 10, 2026·

Viewed 0 times

codereview cpp stackoverflow c++beginner datetime

zodiacsimpleprogramsign

Problem

This is a simple program that can tell you your zodiac sign (Chinese or Western) and the year in which you were born. I made this using very basic beginning knowledge with C++ as practice.

I'm looking for some quick reviews on my code to see if there is anything I'm doing wrong or could be done better, making sure I'm not picking up any bad habits, and that it is easy to read.

#include 
#include 

int main()
{
    int iZodiac;

    time_t     rawtime;
    struct tm* timeinfo;

    time( &rawtime );
    timeinfo = localtime( &rawtime );

    std::cout > iZodiac;

        if (!std::cin || iZodiac  4) {
            std::cin.clear();
            std::cin.ignore(1000, '\n');
            std::cout > iCbirth)) {
                std::cout > iMonth)) {
                std::cout > iDay)) {
                std::cout = 22) {
                    std::cout = 23) {
                    std::cout = 23) {
                    std::cout = 23) {
                    std::cout = 23) {
                    std::cout = 23) {
                    std::cout = 22) {
                    std::cout = 22) {
                    std::cout = 21) {
                    std::cout = 21) {
                    std::cout = 20) {
                    std::cout = 21) {
                    std::cout tm_year + 1900);

            while ((std::cout > iAge)) {
                std::cout << "\nI'm sorry but that is an invalid answer. Please answer using numbers." << std::endl;
                std::cin.clear();
                std::cin.ignore(1000, '\n');
            }
            iBirth = iYear-iAge;
            std::cout << "\nYou were born in the year " << iBirth << std::endl;
        }
//End Year Born Option

    } while (iZodiac != 4);

    std::cout << "\nThank you for playing, Good bye." << std::endl;

    return 0;
}

Solution

-
Since this is C++, prefer std::size_t over C-like size_t. With `, time_t should also be std::time_t. Keep the std::

 consistent for all necessary aspects of the STL.

- 
Declare/initialize variables as close in scope as possible:

int iZodiac;
std::cin >> iZodiac;


This is also an example of Hungarian notation, which is generally discouraged.  Just name it

zodiac

 or something similar.

- 
There's no need for this within the

while

 condition:

while ((std::cout << "\nEnter your current age." << std::endl) /* ... */)


As this isn't a condition, it should just go inside the loop body.  The

std::cin

 should stay, as you're needing to verify that the input was valid.

- 
This program should definitely be modular.  In other words, it should utilize more functions.  The

do-while loop extends through all of main()

, reducing readability and maintainability.

For instance, the menu and input validation should stay in

main(). That can be in a do-while

 loop by itself.  This will prevent the program from shifting control to the other functions until the user selects an appropriate menu choice.  Each choice could could a function (except "quit," which will just fall back to the end of

main()

 for program termination).

- 
All the conditional blocks are confusing to navigate.  One option, although not the best but simple, is with a concise

switch statement. For instance, you could put this into a function that receives zodiac

 and returns the corresponding animal string (not the entire message).

This uses

std::string, which is the C++ STL implementation of a char` array with added features and optimization. I recommend that you become familiar with it.

int zodiac = birth-((birth/12)*12);

std::cout  to use this
        default: throw std::logic_error("unknown zodiac");
    }
}

Code Snippets

int iZodiac;
std::cin >> iZodiac;

while ((std::cout << "\nEnter your current age." << std::endl) /* ... */)

int zodiac = birth-((birth/12)*12);

std::cout << "Your Chinese Zodiac is " << getChineseSign(zodiac);

std::string getChineseSign(const int zodiac)
{
    switch (zodiac)
    {
        case 0 : return "Monkey";
        case 1 : return "Rooster";
        // ...
        case 10: return "Horse";
        case 11: return "Goat";

        // throw an exception if not 0-11
        // include <stdexcept> to use this
        default: throw std::logic_error("unknown zodiac");
    }
}

Context

StackExchange Code Review Q#31652, answer score: 14

Revisions (0)

No revisions yet.