Skip to main content
replaced http://stackoverflow.com/ with https://stackoverflow.com/
Source Link

First, it is a good idea to not include using namespace std;not include using namespace std;. To call a function in a namespace without using it, put the name of the namespace followed by :: in front of the function name, like std::cout << value;

First, it is a good idea to not include using namespace std;. To call a function in a namespace without using it, put the name of the namespace followed by :: in front of the function name, like std::cout << value;

First, it is a good idea to not include using namespace std;. To call a function in a namespace without using it, put the name of the namespace followed by :: in front of the function name, like std::cout << value;

Source Link
user34073
user34073

There are many ways to improve this code other than making it shorter.

First, it is a good idea to not include using namespace std;. To call a function in a namespace without using it, put the name of the namespace followed by :: in front of the function name, like std::cout << value;

Second, use good spacing practices:

void corrected (int &d , int &m , int  &y)
{

 if(d<1)
    {


    d=1 ;

You have extra, unnecessary line breaks, erratic spacing between operators, and even a space before your semicolon. You should use a single space between all operators for ease of reading, one extra line at most between sections, and standard indentation. This is a better version of the above:

void corrected (int &d, int &m, int &y)
{
    if(d<1)
    {
        d = 1;

Third, you are duplicating code left and right in the corrected() function. Why are you nesting your examinations of the day, month, and year variables? Just check them separately, like this:

void corrected (int &d, int &m, int &y)
{
    if(d < 1) {
        d = 1;
    }
    else if(d > 30) {
        d = 30;
    }

    if(m < 1) {
        m = 1;
    }
    else if(m > 12) {
        m = 12;
    }

    if(y < 2000) {
        y = 2000;
    }
    else if(y > 2015) {
        y = 2015;
    }
        
    print24(d, m, y);
}

Notice how I only check each variable once, I don't nest all my checks and make myself check several times based on how the program runs.

Also, it is a good idea to give your variables more descriptive names. It is clear what d, m, and y stand for, but it would be just as easy to write out day, month, and year. Also, corrected() should specify that you are correcting a date, like correct_date().

Also, in a real program, never ever modify a bad input like this - you will confuse your users, and probably chose a wrong value anyway; instead, make the user input the value again.

In valid(), you are just begging for bugs. You should use correct spacing around your operators, braces around the if/else blocks, and indentation; additionally, it would be a good idea to do your checks in the same order, large/small or small/large, not a combination. This is a cleaned-up version:

bool valid (int &d, int &m, int &y)
{
    if (d >= 1 && d <= 30 && m >= 1 && m <= 12 && y >= 2000 && y <= 2015) {
        return true;
    }
    else {
        return false;
    }
}

You should be able to use these tips to rework the rest of your code.