Looking for that healthy criticism(C++)

[Proclivitas]

Alright, so I'm going to add variables to determine yearCheck(->inputCheck() now). This will accept three parameters, 2 for limit and 1 for the user entered variable. This will ensure re-useability. It will unclog a lot of code and I can set global const's for the variables and just change them when needed for things such as acceptable year ranges. This will allow dayCheck() to be hypothetically re-used(but it won't), along with inputCheck(). Is this the direction I should be headed in the spirit of OOP or am I totally off?

[Tamschi]

[QUOTE=Proclivitas;42600267]Alright, so I'm going to add variables to determine yearCheck(->inputCheck() now). This will accept three parameters, 2 for limit and 1 for the user entered variable. This will ensure re-useability. It will unclog a lot of code and I can set global const's for the variables and just change them when needed for things such as acceptable year ranges. This will allow dayCheck() to be hypothetically re-used(but it won't), along with inputCheck(). Is this the direction I should be headed in the spirit of OOP or am I totally off?[/QUOTE]

A bit maybe. It's usually a better idea to avoid globals and just use variables and parameters instead because that way different parts can't interfere.
I don't know what the convention for C++ is though. Personally I'd put the constants at the beginning of the main method, but only if they are "configuration options" and not derived from some implementation limitation.

Passing parameters to yearCheck(...) makes sense, because then you can see the range while reading the main function, but it also means that yearCheck() is probably a bit too trivial to warrant a function. It would be simpler to just have the condition in the main method as the while condition because it's just one short line.

[gparent]

[QUOTE=Dienes;42585893]Types should be chosen as big as necessary and as small as possible. For example "day" will only cover a range from 1 to 31, so an unsigned short (or uint16_t) will suffice (actually an 8-bit type will, but they cause issues with IO streams)[/QUOTE] This advice should be ignored and replaced by "use int for integers and chars for characters". Trust me, by the time you actually want to use a smaller type, you'll know. This program especially is a terrible place for them.

[Proclivitas]

[QUOTE=Jookia;42593671]This is on the topic of writing correct programs. Sure, you're only accepting input from 1950 - 2020, but your dayCheck doesn't know that. Changing yearCheck will break your dayCheck, which seem completely unrelated.[/QUOTE]

-snipped code-

Alright back to working on this tonight after two days of twelve hour work days; and now I'm sick from my stepsister's flu. But alas! I have changed the code around to calculate that leap year. Making everything more Object Oriented. Trying to finish this tonight so I can post the full code for review once more.

Edit: Do I leave in the enum for readability? or take it out?

[Jookia]

Take gparent's advice and ditch the whole 'unsigned short' stuff, it's not needed 90% of the time you program, and worse, it'll make it hard for your functions to interact because of type incompatibilities.
A piece of advice I'd add is that you should use booleans for representing yes/no, rather than just yes. Your 'isLeapYearRef' and 'nonLeapYearRef' can just be 'leapYearRef' that's set to either true or false.

Your code is still incorrect, sorry to say. Just because the month isn't February, doesn't mean the year isn't a leap year. Also February is neither a short month or a long month, which is odd.

[Proclivitas]

I'm only calculating leap year to determine the amount of days in a month; I don't understand the point of determining if every year is a leap year. Please explain

[Dienes]

No, you also give the result of your apparent leap year calculation back to the caller via "isLeapYearRef" (and "nonLeapYearRef"). If this bool is not having the correct value according to how it is named, your function misbehaves. If you do not want to determine if any given year is a leap year, you should not expose this value to the user.

Edit:
Concerning type choice, I think I'm a bit stuck in my work, where the product is time and space critical on a variety of platforms. Still I think one should not be [I]too[/I] wasteful with choosing types in general.

[Proclivitas]

Alright! Finished code is here:
-snipped code-

[editline]23rd October 2013[/editline]

I'm taking out "Enter a value: " in inputValidation because I don't think that should be there, rather in the place where it actually wants you to input a new value. Just leave validation to validation.

[editline]23rd October 2013[/editline]

Actually the whole std::cout part of the inputValidation

[editline]23rd October 2013[/editline]

Alright I revised main, should take a quick look through it again if you read it already

[Dienes]

We are slowly improving, but are not quite there.

What one would expect: "int getMonthLength(int month, int year)" returning the length of a month in a given year.
What it actually does: Setting a bool if it is a leap year, setting another bool if the month is "long" or "short", which is rather vague and returning whatever bool is set first. In addition, whenever one bool is set, it returns and the other bool just stays false.

Move the leapYear out of it into something like "bool isLeapYear(int year)".

Also here:
[cpp]if(!isLeapYear)
while(inputValidation(MIN_MONTH_LENGTH, MAX_NON_LEAP_MONTH, day))
std::cin >> day;

else if(isLeapYear)
while(inputValidation(MIN_MONTH_LENGTH, MAX_LEAP_MONTH, day))
std::cin >> day;

else if(shortMonth)
while(inputValidation(MIN_MONTH_LENGTH, MAX_SHORT_MONTH, day))
std::cin >> day;

else if(longMonth)
while(inputValidation(MIN_MONTH_LENGTH, MAX_LONG_MONTH, day))
std::cin >> day;[/cpp]

the last two else ifs are never reached, because isLeapYear is either true or not, no other possibility.

[Proclivitas]

Alright, new and improved main

-snipped code-

[Dienes]

You seem to be addicted to reference parameters.

"getMonth()" (which btw is a bad name now, because it does not give you a month, it gives you the length of it) should not set two mutually exclusive bools passed in, that's really cumbersome. Why are you not just returning the number of days in the given month/year combination? "int getDaysInMonth(int month, int year) is totally sufficient.

Why does "isLeapYear()" ALSO write the result to a reference parameter? It already returns true or false! "bool isLeapYear(int year)" is sufficient.

"inputValidation()" oh look, a reference parameter! Which is not even written to. Remove that "&" there.

[Proclivitas]

I just realized a fat bug, will fix :3

[editline]23rd October 2013[/editline]

How about this >.<
-snipped code-

[editline]23rd October 2013[/editline]

I really am addicted to reference parameters. So much more fun then just doing blah = func(blah)

[editline]23rd October 2013[/editline]

Looking back at my first post, I can't believe how ugly it looks.

[Dienes]

[QUOTE=Proclivitas;42617643]I really am addicted to reference parameters. So much more fun then just doing blah = func(blah)[/QUOTE]

But their usage is really limited. Prefer return values over out-parameters. Only add outs when the return value is already occupied by something else, like an error code.

The thing is, that they enforce the caller to already allocate a variable beforehand, plus they prevent you from using nesting and chaining, which are common things in this language.

[cpp]// Using out-parameters:
bool leap;
isLeapYear(2013, leap);
if (leap) foo();

// Using regular return values:
if (isLeapYear(2013)) foo();[/cpp]

[cpp]// Using out-parameters:
Date date;
getCurrentDate(date);
int year;
date.getYear(year);
std::cout << "We have the year " << year;

// Using regular return values:
std::cout << "We have the year " << getCurrentDate().getYear();[/cpp]

[Proclivitas]

Alright, new main:

-snipped code-

Everything beautiful now? :3

[editline]23rd October 2013[/editline]

This is in my assignment question: If the user creates a Date object without passing
any arguments, or if any of the values passed are invalid, the default values of 1, 1, 2001
(i.e., January 1, 2001) should be used.

Now this is impossible to even be reached is it not; but you can see I have the default constructor set to January 1st, 2001. Is he just checking to see if we can use a default constructor..?

[Dienes]

Your current program does not fulfill this requirement, because your input validation loops force the user to enter a valid date before a Date object is even constructed.

You would have to move your validation routine into the Date class and perform it upon construction.

[Proclivitas]

Ok, but he gave us pseudocode to follow:

Shows the order in which to declare the Date Obj, which is after I collect data :3

[Dienes]

This page requires a login, but data collection does not mean data validation.

Edit: Okay, then you have to validate without re-prompting the user and just create the 01-01-01 object.

[Proclivitas]

Fixed; [QUOTE=Dienes;42623559]This page requires a login, but data collection does not mean data validation.[/QUOTE]

[editline]23rd October 2013[/editline]

[QUOTE=Proclivitas;42623584]Fixed;[/QUOTE]
So, collect the data, pass it through validation, but don't prompt if it is invalid?

But then theres this aswell: Input Validation: Only accept values between 1 and 12 for the month, between 1 and
31 for the day, and between 1950 and 2020 for the year.

[editline]23rd October 2013[/editline]
-snipped code so i dont get bitched at by prof-