My first program! :D Looking for constructive criticism.

[Zacharee]

So i'm literally only a few days into learning c++ (I took about a semester of computer science in highschool before i dropped out, it taught java though). I decided to write a program that utilizes everything I've learned so far so here it is:

#include <iostream>
using namespace std;

string smonth(int m) {
    string smonth;
    switch(m) {
        case 1 : smonth = "January "; break;
        case 2 : smonth = "February "; break;
        case 3 : smonth = "March "; break;
        case 4 : smonth = "April "; break;
        case 5 : smonth = "May "; break;
        case 6 : smonth = "June "; break;
        case 7 : smonth = "July "; break;
        case 8 : smonth = "August "; break;
        case 9 : smonth = "September "; break;
        case 10 : smonth = "October "; break;
        case 11 : smonth = "November "; break;
        case 12 : smonth = "December "; break;
        return smonth;
    }
}

int main() {
    int dmonth;
    int dday;
    int dyear;
    cout << "Current month: ";
    cin >> dmonth;
    cout << "Current day: ";
    cin >> dday;
    cout << "Current year: ";
    cin >> dyear;
    cout << "\nDate: " << smonth(dmonth) << dday << ", " << dyear << endl;
    int month;
    int day;
    int year;
    cout << "\nMonth of birth: ";
    cin >> month;
    cout << "Day of birth: ";
    cin >> day;
    cout << "Year of birth: ";
    cin >> year;
    int age = dyear - year;
    if(month > dmonth)
        age -= 1;
    else
        if(day > dday && month == dmonth)
        age -= 1;
    cout << "\nBirthday: " << smonth(month) << day << ", " << year << endl;
    cout << "Age: " << age << endl;
}

What this program does is you input the current date, and your birthday and it outputs your age. The reason I post this is to invite constructive criticism. The main things I'm looking for advice on is programming style and structure. Could I have organized this code differently so that it is easier to read? Is there a more efficient way to write this program? Are there any questions here that I dont know to ask?? Again, I'm inviting criticism, so please don't say "well done" or "good job". That does nothing to help me learn.

Edit - ahhhh I didn't read the "read before posting" post! I apologize if I'm in the wrong section. I also apologize if I'm supposed to be asking specific questions (which this certainly isn't... I'm new here!) I changed my title to be more descriptive of the nature of my post.

Edit 2 - WOW I never would have guessed how many more questions this discussion would raise! It has truly been an invaluable learning experience to me, this is better than school! I would like to thank all contributors, you guys are great and you've made me feel so welcome here. I can't even express how much this has strengthened my desire to learn even more. I feel like I'm a single student in a classroom full of teachers. Again, thank you all.

[Bourgond Aries]

I think your code needs a little spacing between what I call "logical chunks":

int dmonth;
    int dday;
    int dyear;

is one such chunk, a declaration chunk. Now put a new line between that and your cin-cout dialogue. That's what I'd do. Makes code easier to read imho.

You also lack indentation on line 48.

Also I use

void sample()
{
}

instead of

void sample(){
}

I find it easier to "line up" the curly braces on the page. Tho many programmers do it like you do, try to experiment and see what fits your own needs.

I'd also get rid of using namespace std; and instead start putting std:: in front of every call that is part of that namespace.

Also return 0; from main ;)

[Zacharee]

Thank you Bourgon Aries. You just raised some more questions, but first here is the edited code:

#include <iostream>
using namespace std;

string smonth(int m) 
{
    string smonth;
    
    switch(m) 
    {
        case 1 : smonth = "January "; break;
        case 2 : smonth = "February "; break;
        case 3 : smonth = "March "; break;
        case 4 : smonth = "April "; break;
        case 5 : smonth = "May "; break;
        case 6 : smonth = "June "; break;
        case 7 : smonth = "July "; break;
        case 8 : smonth = "August "; break;
        case 9 : smonth = "September "; break;
        case 10 : smonth = "October "; break;
        case 11 : smonth = "November "; break;
        case 12 : smonth = "December "; break;
        return smonth;
    }
}

int main() 
{
    int dmonth;
    int dday;
    int dyear;
    
    cout << "Current month: ";
    cin >> dmonth;
    cout << "Current day: ";
    cin >> dday;
    cout << "Current year: ";
    cin >> dyear;
    cout << "\nDate: " << smonth(dmonth) << dday << ", " << dyear << endl;
    
    int month;
    int day;
    int year;
    
    cout << "\nMonth of birth: ";
    cin >> month;
    cout << "Day of birth: ";
    cin >> day;
    cout << "Year of birth: ";
    cin >> year;
    
    int age = dyear - year;
    if(month > dmonth)
        age -= 1;
    else
        if(day > dday && month == dmonth)
            age -= 1;
    
    cout << "\nBirthday: " << smonth(month) << day << ", " << year << endl;
    cout << "Age: " << age << endl;
    
    return 0;
}

Regarding "using namespace std;" I'm not entirely sure what that actually does. Which calls use the std namespace? The book I'm using to teach myself is "Thinking in C++, 2nd ed. Volume 1" by Bruce Eckel, he does this in all of the example programs in the book. Why do you prefer putting std:: in front of every call in that namespace? And you're right about the brackets, lining them up certainly makes it easier to read, but I think the style I'll use will depend on the circumstances of my programming. Example, if I have a tight deadline for something and I have to code fast, i'll probably use

void sample() {
}

because in my ide (Code::Blocks) the closing bracket is insterted automatically, so after typing the opening bracket all I have to do is hit [enter][enter][^][tab] and start coding. On the other hand if I'm doing a collaborative project where readability is important I'll probably use

void sample()
{
}

Last question: What does "return 0;" do?

[Chervil]

There are a few problems. The function smonth() should return a value. However the return statement is located at a point where it can never be executed.

Also I tried to compile the program, which failed because #include <string> was missing. When I added this and ran the program, it actually crashed.

(I had intended to comment on the style, but I'd fix the outstanding issues first).

[Olysold]

C++ reserves certain words for its stuff in the standard library, ie. cin and cout. By declaring using namespace std; you eliminate the need to type out std:: anything, and that may lead to unintentional usage of certain functions, classes or whatnot from the standard library.

You don't have to use std:: throughout your code, you could just specifically declare things that you want to use. Instead of using namespace std; you could use using std::cin; using std::cout; using std::endl;

void sample() {
}

void sample()
{
}

These are really just stylistic choice and a matter of personal/team preference. Each style even has their own name, so I guess it's an indication that both styles are used.

return 0; is an indicator that your code runs fine. When you program returns 0, it means the program managed to get all the way to the bottom. If it returns anything else, there's an error. You won't see a need for it at the moment (neither do I, still a scrub) but it's a matter of good practice I suppose.

[Bourgond Aries]

return 0;

Ask yourself why there is an int in front of the main implementaton: int main(){...}

It's because this function will "return" an integer. This integer can be from -2147483648 to 2147483647 on 32 bit machines.

Every function in C++ has a return type. If you do not want to return anything, you call it a void: "void function(){...}"

A use of the return type is for example in this code:

int powerof(int number, int power)
{
    for (int i = 1; i < power; i++)
        number *= number;
    return number;
}

You can now conveniently call the code from another function:

int main()
{
    std::cout << powerof(32, 2);
    return 0;
}

This will print the number that powerof returns.

[Zacharee]

@Chervil That is odd... I originally had "#include <string>" in there but I took it out just to experiment and it ran fine without it so I (perhaps erroneously) concluded it wasn't necessary in this circumstance. The program runs fine for me, in fact it kept crashing until I added the return statement where it is. Perhaps it is a difference in compilers? (not that I know what I'm talking about ^.^) Which compiler do you use? I'm using the one included in Code::Blocks.

@Olysold I believe you're right about "good practice" that is precisely the reason I posted this topic. Not because I'm actually having issues with my code but because I'm a perfectionist and I want to establish good habits while I'm still learning. as for "return 0;" i didn't seem to make any difference to the execution of the program after i recompiled it

[Zacharee]

I think I'm starting to get it... so the return 0; is there for situations where you're actually using main as a function??? Or am I way off the mark?

[Chervil]

Regarding the header file, some compilers may include one header automatically when another is used (presumably because the header itself has further #include statements).

When I use code::blocks I get the following message:

...\main.cpp||In function 'std::string smonth(int)':|
...\main.cpp|21|warning: control reaches end of non-void function [-Wreturn-type]|

That is to say, it is indicating that it is possible to reach the end of the function without returning any value.

The program behaves a little differently using code::blocks, but it would be wrong to depend upon such quirks for the correct operation of the program.

The line return smonth; should be moved outside the switch block, so it is the last line before the closing brace of the function. (Incidentally, as a matter of style, I'd suggest the variable name is made different to the name of the function).

[Olysold]

I'm using code::blocks too. Right above "Press any key to continue" there's "Process returned 0". That's where it is returning 0. Try changing the number after return and it will change as well in the cmd prompt.

Beyond that you'll need an actual programmer to tell you why return 0; is important. My best guess is sometimes the program will be able to compile, but didn't actually manage to get all the way through, resulting in the program not returning 0.

[Zacharee]

ahh i didn't even notice my return was in the switch block. ok i moved it, also i changed the name of that function to "date"

[Chervil]

Just a brief comment on compiler warning messages (which I presume were also shown in your version).

It's a good idea to make a habit of checking and correcting the code causing all warnings. If there are just one or two messages, such as "comparing signed with unsigned value" or something like that (just an example), it's easy enough to just carry on without any problems. But, when working on a larger program, there may be hundreds of such warnings, and its possible that a really important one may slip by unnoticed. So as a matter of routine its good to resolve all the messages.

Basically my outlook is that the compiler is a tool which is there to help, any messages it spits out are to be considered beneficial, as they can help avoid making some kinds of mistakes.

[Zacharee]

so far my outstanding questions are:

I think I'm starting to get it... so the return 0; is there for situations where you're actually using main as a function??? Or am I way off the mark?

and

Why do you prefer putting std:: in front of every call in that namespace?

Thank you everyone who replied. I realize this is what the welcome topic would call an "open-ended time sink" so double thanks for all of your patience with me and and my undoubtedly endless questions :)

[LB]

@first question
main is not is not a function, it's just easier to pretend that it is. The return statement in main has the effect of exiting the process and giving the system the exit code specified in the return statement.

@second question

http://www.parashift.com/c++-faq/using-namespace-std.html

http://stackoverflow.com/questions/1452721/why-is-using-namespace-std-considered-a-bad-practice-in-c

[Zacharee]

i see... so it can cause conflicts when using multiple namespaces. but it be acceptable to use using namespace std; only for std and then use full names for anything else??

[LB]

That is like hanging out with the criminals that steal candy but not the criminals who steal from banks.

It's just a bad habit that you should get out of. Also, you never know what might be in std that you don't even know about...

[Chervil]

I remember seeing a case where someone had defined a function called swap(). As it happened there was an error in the code and the function didn't work (can't remember the exact scenario).

However, the program as a whole still seemed to work correctly. Why? Because there was a version of swap() in the std namespace, which was being used without the programmer even being aware of it.

That may seem an isolated example, but it illustrates that having "using namespace std;" can cause unexpected consequences, without it being obvious that it has even happened.

[LB]

There's also std::count, std::left, std::list, std::array, etc. basically a lot of names that might get used instead of what you intended.

[SamuelAdams]

There are no comments at the top to tell you what the program does. today that may not be important, but in 2 months or 2 years. you also have no comments anywhere, for example:

if(month > dmonth)

without looking at the code, in a year, you just like everyone else, will have no idea what month and dmonth represent.

---
I also don't see any checks to make sure that the date entered is valid.
So if the user enters 13 for the month, the program just accepts that, even though it's wrong.

---

While it may not be desired, since you can enter a date that is not (today).
There is no reason for you to have the user put in the current date, since you can just get that from the computer. Not really a problem with your code, just a user friendly suggestion.

---
Last, when I declare int x; I like to set the value, else the computer uses memory that may already have a value.
for example, try:

    int dmonth;
    int dday;
    int dyear;
    cout << dmonth << " " << dday << " " << dyear << endl;

Then try:

    int dmonth=1;
    int dday=1;
    int dyear=1900;
    cout << dmonth << " " << dday << " " << dyear << endl;

[LB]

@SamuelAdams

A program should be written in such a way that you can easily identify exactly what it does and what its purpose is just from reading the code. If comments are needed to explain something, your code is poorly designed.