The way I Code!

You should split it up into separate files to make it easier to manage. Also I noticed that you use a comparison guard to keep hunger, boredom, and lazy above 0. May be better to put the comparison gaurd where it actually decreases/increases the values.

What you've got here is too small of a project to truly benefit much from OOP. However, what you do have is nicely OOP-ed.

Since it's not large enough to really critique your OOD, I'll give some comments on general code. A minor nitpick (really an opinion) is that I like to have newlines at the end of a line, not at the start of a line (especially since in your main menu, they are at the end of the lines, where as the rest is at the start). I find it more readable that way, but it doesn't really matter. More important, might be some simple comments in the code. Your code is quite readable already (thanks to your consistent and logical naming of variables, functions, and classes), but some small comments could be helpful. One such place might be where you randomly select from a couple strings to say. A one-line comment could save a bit of thought there. You might want to make some const variables to replace some of your constants. Like the thresholds for different levels of being happy.

Overall, quite nice, but it would take a larger project to really see how you code. Great job.

There isn't any major problems with your code, I noticed a couple of things though:

The user have no idea what hunger, lazy and boredom actually represent. For instance I would have thought that high values meant that the pet was hungry, lazy and bored. You should encapsulate these into some class. At the same time you should try to get away from plain int, noone knows what it means to add 4 to boredom, but if you instead have a constant called IncrediblyBoringActivity and multiply it by TimeDoingIt, then you would have very readable code. Also IncrediblyBoringActivity could be an integer or an object of a class. Generally it's a good idea to get away from all those "magic literals". In the same way you should somehow abstract away time as an int object, and instead make a type Seconds (maybe just a typedef for int or float). We have no idea if 4 is 4 days or 4 ms.

You never consider the fact that your objects could overflow, if passed big enough values.

Check doesn't really tell you what is done, it could check if the Pet is dead, or if it has peed on the floor, noone knows without looking at the implementation (that shouldn't be necessary). Also it doesn't only check the mood, it also transforms it into a string. One function should only do one thing. I think you should refactor it into two functions, one public one called GetMoodDescription and another, private one, called UpdateMood. Also you computer the m_mood variable in Check, you should have a seperate function for this also, because at some point in the future you might want to change the way m_mood is calculated.

You place the definition of Pause in the middle of Pet's functions' definitions, you should move it to its own place so it won't be confused with Pet's functions.

random_shuffle is made to shuffle an entire container, you should really use a function to choose a random element in the container instead (maybe using rand()). But it's actually the opposite.

system("cls"); will only work on Win32 systems. There is no proper C++ way of doing this, but I think you should instead print 40 empty lines or something like that. At least check if you're on a platform where it's supported. Also move it into its own function so the client code doesn't need non-cross-platform code.

Should bored really be a parameter of the Talk function? You should be able to ask Pet to talk, but you can't tell it how boring the conversation will be. Maybe you need a conversation object, were one of the members is a boringness object. (the same thing about Play)

There are lots of things which should be encapsulated, for instance you should have a Burp function instead of just writing burp. What if you write burp 15 places in your code, and then suddenly you feel the need to remove two !, or maybe you need to increase the smell of the pet when it burps. This is just one thing, you have many cases like this one.

Maybe you should add some parameters to the Feed function to tell what, and how much to give it. And to nitpick, it burps before its hunger is decreased.

To summarize it all: Get rid of magic values and make us of the abstraction facilities you have in C++.

These are just some general advices, most will be a total overkill in this case, but OOP wasn't made for programs of this size. If you assigned AI to pet, voice synthesis and graphics, then you would see the benefits of OOD.

thanks for the feedback! I will work on everythang [smile]

Looking at things to a different angle than others have looked at thus far...

Your if statements testing m_mood are a bit of a problem.
you check if m_mood is >= 5 and then check if it is < 5. If it isn't >= 5 so it must be < 5 so there's no point checking if it is. This also means that it will never hit your <=0 case because < 5 will always succeed first.

This is also an appropriate place to use a switch statement:

if (m_mood < 0)  mood = "Sad and Angry";else  switch (m_mood / 5) {    case 0: //0..4        mood = "Angry";    break;    case 1: //5..9        mood = "Sad";    break;    case 2: //10..14        mood = "Fine";    break;    case 3: //15..19        mood = "Happy";    break;  }

Also, I would always use a "do { } while;" for loops that I know must always execute at least once.

Overall, your code looks quite tidy, and much of what you've done is pleasing.

Edit: Stuck code in the switch statement.

Quote:

Original post by iMalc Looking at things to a different angle than others have looked at thus far... Your if statements testing m_mood are a bit of a problem. you check if m_mood is >= 5 and then check if it is < 5. If it isn't >= 5 so it must be < 5 so there's no point checking if it is. This also means that it will never hit your <=0 case because < 5 will always succeed first. This is also an appropriate place to use a switch statement: switch (m_mood / 5) { case 0: //0..4 break; case 1: //5..9 break; case 2: //10..14 break; case 3: //15..19 break;} I would always use a "do { } while;" for loops that I know must always execute at least once. Overall, your code looks quite tidy, and much of what you've done is pleasing.

Thank you very much [smile]

i dunno about u guys, but one of the first things i do when i install visual studio is turn off auto indent and insert tabs. but other than that seems ok. not indenting public seems kinda odd... but to each his own!

Quote:

Original post by yadango i dunno about u guys, but one of the first things i do when i install visual studio is turn off auto indent and insert tabs. but other than that seems ok. not indenting public seems kinda odd... but to each his own!

not indenting public is kind of weird to me too, for some strange reason vc++ does it automatically.

Quote:

Original post by NUCLEAR RABBIT Quote: Original post by yadango i dunno about u guys, but one of the first things i do when i install visual studio is turn off auto indent and insert tabs. but other than that seems ok. not indenting public seems kinda odd... but to each his own! not indenting public is kind of weird to me too, for some strange reason vc++ does it automatically.

Pretty common practice, I like it that way, keeps the distinction between what I care about and the internal stuff clear, when I'm the user of the class.

Apart from that your code looks great, apart from the details already mentioned by others. Keep it going that way