Attempting a "Simple" Vector-Based Inventory Test

Started by
16 comments, last by Khatharr 11 years, 5 months ago

Using find is also not needed as you know which item you need to check, even if you get it working (I believe that if you use Strings, aItem is a struct and they are the first element declared in the struct it will work as intended), you will be tossing away processing (as you need to find the element in the array).

Actually his implementation will always work regardless of where the string is stored in the same struct as he is providing an == operator which find will call for the data type you give it.

Also another tip: instead of doing index++, try to use ++index. In this case it doesn't matter but it is a good habit to have, so when you only want to increment and decrement a variable by 1 ever and nothing else write ++var or --var. The reason behind this is that the ++operator or --operator(both pre and post increment or decrement) can be overloaded and do a lot of work that is unnecessary in that particular case.

Worked on titles: CMR:DiRT2, DiRT 3, DiRT: Showdown, GRID 2, theHunter, theHunter: Primal, Mad Max, Watch Dogs: Legion

Advertisement

The code is artificial, and more importantly, it doesn't do anything (except printing stuff). Thus there's very little that could be wrong with it on low level, and in the absence of context it's hard to say anything about design issues.

Just some cosmetic things I'd point out to a student:
- "a" is useless because you could be printing "index+1" instead
- any variables like "choice" should be defined as close as possible to where you actually use them for the first time
- instead of writing "choice-1" everywhere, sanitize the number once to the value you actually want (-=1)
- finally, magic numbers like "01" and "02" are generally bad; in a real program you would likely use an enum and descriptive labels like ITEM_HEALTHPOT or ITEM_FIREPOT

Which compiler are you using? If it's new enough to have some C++11 features, they can make your life easier in small (and occasionally big) ways.


Thank you very much. I realize the artificial nature. This is a glorified snippet that is mostly designed to teach me how to use and modify vectors. The suggestions I was looking for are exactly the ones you mentioned- just good housekeeping and dev habits in general. I'm learning this myself, so I'm not able to get the benefit of a teacher standing over me going "No- that won't work in six months when you're doing something better."

I'm using Microsoft VS 2012- I'm not sure what "C++11 features" are, but if they make my life easier, I'd certainly be interested in hearing about them!

EDIT: I've applied the changes you suggested, and the code reads much better.

EDIT 2: The next step for me is to turn this into a class, I suppose!
What do you mean by turning it into a class?

Also, please don't place extended logic inside switch statements. It's a nightmare to read. Check this out:

[source lang="cpp"]
void processItemSelect() {
int choice = -1;

while(choice == -1) {
cout << "Which item would you like to use?" << endl;
for(vector<aItem>::size_type index = 0; index < inventory.size(); ++index) {
cout << "Item " << index + 1 << ": " << inventory[index].itemName << endl;
}
cout << "MAKE YOUR CHOICE." << endl << "Choice: ";
cin >> choice;
--choice;
if((choice >= inventory.size()) || (choice < 0)) {
choice = -1;
cout << "Choice out of bounds. Stop being a dick." << endl << endl;
}
}
switch(inventory[choice].itemType) {
case ITEM_POTION:
cout << "You used a healing potion!" << endl;
break;
case ITEM_BALLS:
cout << "FIERY BALLS OF JOY!" << endl;
break;
default:
cout << "Invalid Item type" << endl;
break;
}
}

int main() {
int selection = 0;
cout << "This is a test game to use inventory items. Woo!" << endl <<
"You're an injured fighter in a fight- real original, I know." << endl <<
"1) Use an Item. 2) ...USE AN ITEM." << endl;
cin >> selection;

switch (selection) {
case 1:
processItemSelect();
break;
case 2:
cout << "Why do you have to be so difficult? Pick 1!" << endl;
break;
}

return 0;
}[/source]

Finally, do not prefix numeric literals with a zero. That's the notation for the octal numbering system:

10 is 10.
010 is 8.
void hurrrrrrrr() {__asm sub [ebp+4],5;}

There are ten kinds of people in this world: those who understand binary and those who don't.

I'm using Microsoft VS 2012- I'm not sure what "C++11 features" are, but if they make my life easier, I'd certainly be interested in hearing about them!
C++11 is the newest C++ standard. VS2012 supports about half of the new features. Most of the features are not relevant to a beginner, but some of them are seriously worth learning immediately. Together, they almost feel like you are writing in a different language.

The auto keyword (lets you type "auto" in many situations where you'd otherwise have to type something painful like "std::vector<std::pair<int, myType>>::iterator_type")
http://en.wikipedia....#Type_inference
Range-based for loop (use whenever you can):
http://en.wikipedia....-based_for-loop
nullptr (use whenever you can):
http://en.wikipedia....ointer_constant

Lambdas are a bit more obscure than the above, but not actually hard. If you have the capacity, check them out. Standard containers like vector, standard algorithms and lambdas together are magic.
http://en.wikipedia....and_expressions

Smart pointers, unique_ptr and smart_ptr: you *have to* eventually learn these or you won't be coding good C++11. However, you don't need to touch them at all until you start using "new" and start seeing naked pointers like int* or SomeType* in your code. Naked pointers are evil. Smart pointers get rid of them.
http://en.wikipedia....i/Smart_pointer

The biggest thing that you unfortunately don't have yet in VS2012 is uniform initialization.

What do you mean by turning it into a class?


Well, this standalone exercise was standalone because I realized that I had no idea how to create an inventory system- this is all written from scratch (I'm still enough of a beginner to feel like a "Master Programmer" for getting it to work rolleyes.gif ). I actually have a very basic text RPG with some functional abilities, and wanted to isolate this painful learning experience from the working code! The end result of this is to create a class with functions that I can call from within the game code, in order to perform inventory functions without cluttering up the main .cpp.
Gaah, 2 posts while I was editing. Derp.

Ah, I see. You just mean implementing it. I'm glad to hear the you're enjoying your successes. smile.png

Edit - Occurred to mention it while I was outside - You would probably be better off placing the usage message in the item's effect processing. That way you just have to proc the item effect of the selected item instead of switching to print the usage message.
void hurrrrrrrr() {__asm sub [ebp+4],5;}

There are ten kinds of people in this world: those who understand binary and those who don't.

Gaah, 2 posts while I was editing. Derp.

Ah, I see. You just mean implementing it. I'm glad to hear the you're enjoying your successes. smile.png

Edit - Occurred to mention it while I was outside - You would probably be better off placing the usage message in the item's effect processing. That way you just have to proc the item effect of the selected item instead of switching to print the usage message.


Thank you! :)

I see... and thank you for your code example- that looks much more friendly than what I'm hacking together! Got a lot to learn, but getting there...
As long as you keep going you get 'there' in the end. ;)
void hurrrrrrrr() {__asm sub [ebp+4],5;}

There are ten kinds of people in this world: those who understand binary and those who don't.

This topic is closed to new replies.

Advertisement