vector.size() not working
Ok, I am ALMOST finished with this inventory but one last little thing is giving me problems: On line 49 I am attempting to show the number of items in my inventory. Each item consists of a number and a name. Using Names.size() worked just fine until I added i++ to the loadfile() function (line 115). How do I fix Names.size() so that if will output the correct number of elements in the vector Names, and therefore the number of items stored in my inventory? THANK YOU!! :)
|
|
Last edited on
What is happening?
I can pretty much guess - but if you actually tell us of the problems you are having - rather than - this is my code - it is not working - please fix it
I can pretty much guess - but if you actually tell us of the problems you are having - rather than - this is my code - it is not working - please fix it
| Using Names.size() worked just fine until I added i++ to the loadfile() function (line 115). |
Why did you do that? I don't see i used for any useful purpose in the loadfile function. You are simply incrementing i over and over but you never use it. It has nothing to do with the number of elements in the vector.
I don't know how to answer the original question without more detail but I do see a few things worth noting.
The following functions should receive const references to the vectors in order to avoid changing them accidentally.
saveInventory, searchItems, listItems
performSelection should be coded using if..else if... else pattern. Once a selection has been handled there is no point performing the remaining checks.
I don't see any really obvious problems but I'm sure that the std::vector::size function works fine. You need to debug the program and try different combinations of things until you can figure out how the object is being changed to have more or less elements from what you expect. I would change the listItems function to print the size of both containers. Try running the program and always list the items in between every other command and see if you can pinpoint where the object is being changed unexpectedly.
The following functions should receive const references to the vectors in order to avoid changing them accidentally.
saveInventory, searchItems, listItems
performSelection should be coded using if..else if... else pattern. Once a selection has been handled there is no point performing the remaining checks.
I don't see any really obvious problems but I'm sure that the std::vector::size function works fine. You need to debug the program and try different combinations of things until you can figure out how the object is being changed to have more or less elements from what you expect. I would change the listItems function to print the size of both containers. Try running the program and always list the items in between every other command and see if you can pinpoint where the object is being changed unexpectedly.
The problem is the loadInventory function but it has nothing to do with the unused i variable. It is reading the last item twice therefore a duplicate item gets inserted into each vector. I can't say that I understand why that is happening but that is the problem. I noticed that when the file is created by saveInventory you write a final endl which causes the file to have a blank line at the end. When reading the file you are looping until the EOF is reached. Since after the last item is reached there is a blankline, the loop doesn't end when you expect and for some reason the program ends up re-reading the last item again. Perhaps someone who writes these sort of file i/o problems can suggest a solution. I don't have time to fiddle with it any longer to be honest.
It isn't reading the last item twice, but it is failing once. The problem is the way the input loop is designed: it doesn't check for validity before appending items to the vectors... and failed inputs won't change the targeted item -- which still has the previous input value. Hence, the last two items are identical.
There are a number of issues with the code that I'll address in a moment. But first, the modified code:
(continued due to post size)
There are a number of issues with the code that I'll address in a moment. But first, the modified code:
(continued due to post size)
|
|
First off, starting at line 1: the <cstdlib> file is only needed for three things: system(), which is evil; exit() which is also not recommended; and EXIT_SUCCESS, which I personally like but is not worth including a whole file where
As pertains to the use of system(), see lines 45-46 for the 'corrected' method. This requires the <limits> header. This correction also exposes an input design failure: don't use formatted input functions (which in C++ are the >> operator functions) when dealing with the user. Remember, the user always expects to press ENTER after every input. Your input methods should match. You'll notice that the first thing I did was go through the code and add a
However, I have added a suggested input function for reading integers from the user: see lines 51-77. Again, user input is always line-oriented. After reading the entire line, a temporary istringstream is used to read the integer, check that it worked, and complain if it didn't. Ultimately, the user must input an integer value to continue.
Next I'll touch on the changes to the function prototypes I made on lines 11-17. This all has to do with scope and visibility. As it was, many of the arguments to the functions were simply used as local variables inside the function. For example, the loadfile() function took as argument a reference to a string to use when asking the user what filename to open. Since the loadfile() function does all necessary I/O with the user and the name of the file opened is not needed elsewhere, there is no need to have the filename variable visible anywhere outside the loadfile() function. Hence, I made it a local variable and removed it from all places elsewhere. Similar changes were made with other functions. Notice also how this unclutters the main() function with unused variables: see lines 24-27.
The menuSelection() function's obvious purpose is to return the number of the user's selection. This is a good example of making things look like what they do:
Hence, I modified the function to work the same way.
The change to the performSelection() is similar, considering lines 39-42: it returns whether or not to stop the program. Notice also how loadfile() returns success or failure instead of simply exit()ing on failure.
This is also an important point. Before, you had three distinct exit points for the program. Don't do that. Unless there is a catastrophic failure, exit points should be consolidated as much as possible, and kept as close to main() as possible. (This is my opinion.)
The problem with multiple exit points is that duplicate code is spread around. Keep things simple and in one spot as much as is reasonable. The changes I made always return an OK code to the OS, but you can add in a failure exit if you wish.
performSelection(), lines 96-125: If you are selecting on constant integer values, avoid if..else... constructs and use a switch instead.
A change I did not make, that you should consider, is using an enumeration to name the values returned by menuSelection() and listed in the switch statement of performSelection(). This makes it easier to make changes to the menu -- you no longer have to care what the exact number is, just change the enum name and use it to your advantage in the switch.
loadfile(), lines 152-166: Here is the corrected file input. Notice how if there is any input failure at all neither the 'Numbers' nor the 'Names' vectors are modified.
All other changes should be fairly obvious. There are other things I would have done differently, but you must work through them yourself. Hopefully this is enough to get you started. (The program works correctly now, at least.)
Hope this helps.
return 0 (lines 47-48) suffices.As pertains to the use of system(), see lines 45-46 for the 'corrected' method. This requires the <limits> header. This correction also exposes an input design failure: don't use formatted input functions (which in C++ are the >> operator functions) when dealing with the user. Remember, the user always expects to press ENTER after every input. Your input methods should match. You'll notice that the first thing I did was go through the code and add a
cin.ignore( numeric_limits <streamsize> ::max(), '\n' ); after every cin >> someinteger;. Likewise, all cin >> somestring; have been changed to getline( cin, somestring );, which is more appropriate.However, I have added a suggested input function for reading integers from the user: see lines 51-77. Again, user input is always line-oriented. After reading the entire line, a temporary istringstream is used to read the integer, check that it worked, and complain if it didn't. Ultimately, the user must input an integer value to continue.
Next I'll touch on the changes to the function prototypes I made on lines 11-17. This all has to do with scope and visibility. As it was, many of the arguments to the functions were simply used as local variables inside the function. For example, the loadfile() function took as argument a reference to a string to use when asking the user what filename to open. Since the loadfile() function does all necessary I/O with the user and the name of the file opened is not needed elsewhere, there is no need to have the filename variable visible anywhere outside the loadfile() function. Hence, I made it a local variable and removed it from all places elsewhere. Similar changes were made with other functions. Notice also how this unclutters the main() function with unused variables: see lines 24-27.
The menuSelection() function's obvious purpose is to return the number of the user's selection. This is a good example of making things look like what they do:
y = foo();Hence, I modified the function to work the same way.
The change to the performSelection() is similar, considering lines 39-42: it returns whether or not to stop the program. Notice also how loadfile() returns success or failure instead of simply exit()ing on failure.
This is also an important point. Before, you had three distinct exit points for the program. Don't do that. Unless there is a catastrophic failure, exit points should be consolidated as much as possible, and kept as close to main() as possible. (This is my opinion.)
The problem with multiple exit points is that duplicate code is spread around. Keep things simple and in one spot as much as is reasonable. The changes I made always return an OK code to the OS, but you can add in a failure exit if you wish.
performSelection(), lines 96-125: If you are selecting on constant integer values, avoid if..else... constructs and use a switch instead.
A change I did not make, that you should consider, is using an enumeration to name the values returned by menuSelection() and listed in the switch statement of performSelection(). This makes it easier to make changes to the menu -- you no longer have to care what the exact number is, just change the enum name and use it to your advantage in the switch.
loadfile(), lines 152-166: Here is the corrected file input. Notice how if there is any input failure at all neither the 'Numbers' nor the 'Names' vectors are modified.
All other changes should be fairly obvious. There are other things I would have done differently, but you must work through them yourself. Hopefully this is enough to get you started. (The program works correctly now, at least.)
Hope this helps.
Topic archived. No new replies allowed.