Well... long story short I had another thread about a month ago ? maybe a little later.. I went through the design process like.. 10 times and now I think I finally have my final design pretty pat..
I'm only having one simple issue.. I have a class Card that holds and int Face(1 ace, 2 two, 13 king) and int Suit (1 heart, 4 spade). Player hands are a list of card objects, same goes for the deck..
Code:
list <Card> Contents..
list <Card> Hand
I want to pushback a copy of a card from the deck, and put it into my hand..
Code:
list <Card>::iterator i = Deck.Contents.begin();
**** iterate through list *****
this->Hand.pushback(*i);
It seems to work for one iteration but .. only one...
Does anyone have any ideas? I've looked everywhere...
I tried allocating a new card new Card (suit, face) and trying to copy it over insert..
but I'm a little lost...
edited to add source.
Last edited by exiledgolem; November 24th, 2009 at 12:28 PM.
Reason: added source...
error C2664: 'std::list<_Ty>::push_front' : cannot convert parameter 1 from 'std::list<_Ty>::_Iterator<_Secure_validation>' to 'const Card &'
Code:
void Player::PickupCards(const int NumberOfCards) // pickup a certain amount of cards from our global deck
{
EndGame(); // check to see if the deck vector is empty, ifso endgame, display winner
int Count = 0; // counter to end loop when == numberofcards we want to take
for (list <Card>::iterator i = Deck.Contents.begin(); i != Deck.Contents.end(); ++i) // go backwards....
{
Hand.push_front(i); // here
thank you all , I found out it was such a simple fix
before code..
Code:
void Player::PickupCards(const int NumberOfCards) // pickup a certain amount of cards from our global deck
{
EndGame(); // check to see if the deck vector is empty, ifso endgame, display winner
int Count = 0; // counter to end loop when == numberofcards we want to take
for (list <Card>::iterator i = Deck.Contents.begin(); i != Deck.Contents.end(); ++i) // go backwards....
{
Hand.push_front(*i); // take the last value from the deck, and put it into that players hand
Deck.Contents.pop_front(); // remove the last value of the deck, shortening it, so value was removed
EndGame(); // since we removed a value, make sure the deck still has values in it
// so we can draw cards
++Count; // ifso, increase count
if (Count == NumberOfCards) // if count == numberofcards
break; // stop picking cards up
}
}
after code
Code:
void Player::PickupCards(const int NumberOfCards) // pickup a certain amount of cards from our global deck
{
EndGame(); // check to see if the deck vector is empty, ifso endgame, display winner
int Count = 0; // counter to end loop when == numberofcards we want to take
for (list <Card>::iterator i = Deck.Contents.begin(); i != Deck.Contents.end(); ) // go backwards....
{
Hand.push_front(*i); // take the last value from the deck, and put it into that players hand
++i;
Deck.Contents.pop_front(); // remove the last value of the deck, shortening it, so value was removed
EndGame(); // since we removed a value, make sure the deck still has values in it
// so we can draw cards
++Count; // ifso, increase count
if (Count == NumberOfCards) // if count == numberofcards
break; // stop picking cards up
}
}
I just had to move the ++i after I copied the value...
void Player::PickupCards(const int NumberOfCards) // pickup a certain amount of cards from our global deck
{
//...
for (list <Card>::iterator i = Deck.Contents.begin(); i != Deck.Contents.end(); ++i) // go backwards....
{
Hand.push_front(i); // take the last value from the deck, and put it into that players hand
Deck.Contents.pop_front(); // remove the last value of the deck, shortening it, so value was removed
//...
}
}
When you do a pop_front you invalidate the iterator i. Therefore, you have undefined behaviour.
Cheers, D Drmmr
Please put [code][/code] tags around your code to preserve indentation and make it more readable.
As long as man ascribes to himself what is merely a posibility, he will not work for the attainment of it. - P. D. Ouspensky
for (list <Card>::iterator i = Deck.Contents.begin(); i != Deck.Contents.end(); ++i) // go backwards....
{
Hand.push_front(*i); // take the last value from the deck, and put it into that players hand
Deck.Contents.pop_front(); // remove the last value of the deck, shortening it, so value was removed
EndGame(); // since we removed a value, make sure the deck still has values in it
// so we can draw cards
++Count; // ifso, increase count
if (Count == NumberOfCards) // if count == numberofcards
break; // stop picking cards up
}
you invalidate the iterator!
edit: posted before refreshing :-/
Last edited by Amleto; November 24th, 2009 at 01:11 PM.
Hi! I remeber helping you when way back. Good to see you kept working on your program.
Looks like you found your problem, which is good, but I'll still tell you what I think about some parts of your code:
StandardDeck::Shuffle. You are aware that algorithm::random_shuffle does the same thing? You are re-writing the wheel on that one. Not to mention you also re-invented the swap function. Take a look here. http://www.cplusplus.com/reference/a...andom_shuffle/ it's not very complicated either.
Your Card::toString. I remember with Paul telling you to use an array, like this:
Something like that. I'm not going to re-write the entire code. But using this kind of solution is generally more maintainable, and has better performance.
Avoid doing this:
Code:
class MyClass{
...
}gloableName1, globaleName2
Where there is nothing wrong with doing it that way, has some problems:
- 1. People just plain aren't used to this kind of construct.
- 2. It's not as clear as having an area right before main labeled "//Global Variables"
- 3. You won't be able to move your classes away into their own .h
Hi! I remeber helping you when way back. Good to see you kept working on your program.
Looks like you found your problem, which is good, but I'll still tell you what I think about some parts of your code:
StandardDeck::Shuffle. You are aware that algorithm::random_shuffle does the same thing? You are re-writing the wheel on that one. Not to mention you also re-invented the swap function. Take a look here. http://www.cplusplus.com/reference/a...andom_shuffle/ it's not very complicated either.
Your Card::toString. I remember with Paul telling you to use an array, like this:
Something like that. I'm not going to re-write the entire code. But using this kind of solution is generally more maintainable, and has better performance.
Avoid doing this:
Code:
class MyClass{
...
}gloableName1, globaleName2
Where there is nothing wrong with doing it that way, has some problems:
- 1. People just plain aren't used to this kind of construct.
- 2. It's not as clear as having an area right before main labeled "//Global Variables"
- 3. You won't be able to move your classes away into their own .h
There. The code looks good!
Thanks for the comments.
As for the random_shuffle routine... I use lists, and it's not available for lists? Unless I'm just missing it when I type in Deck.Contents....it's just not there.. You are right about swap though.. I just thought I would exercise that little ol'brain of mine... considering random_shuffle wasn't there.
Long story short as for that toString function... I tried going about doing it paul's way, and for some reason as for which I do not remember, it just seemed like it didn't "fit" well with me. It might have been a design flaw on my part, which I believe is fixed now, but I don't really care about the speed of it. I'm more concerned as to just get this thing out and working to finally say I've finished something on my own.
I actually just went to the bookstore recently and bought a book on general algorithms and their everyday use in programming. Great book. I know it'll help me down the line on any speed inprovements I might want to take a look at if I get into anything serious.
As for the global objects, I was doubting myself too when it came down to my methods. I'll take your advice mainly because you are able to distinguish it easier.
Thanks again everyone for the great advice!
I still need to learn as to why my iterators are becoming invalidated in certain situations... I still have a problem with my looking for card from another player function
Code:
void Player::CheckFor (const int face, Player& pplayer) // need set value 1 for twos.. 13 for aces, 4 for fives
{
list <Card>::iterator i;
for (i = pplayer.Hand.begin(); i != pplayer.Hand.end(); ++i) // for that players whole hand
{
if ((*i).Face == face)
{
Hand.push_front(*i);
i = pplayer.Hand.erase(i);
}
}
}
.. I'm using the exact same methods as the draw card function but I run into problems eghh... *sigh*...
Last edited by exiledgolem; November 24th, 2009 at 08:52 PM.
As for the random_shuffle routine... I use lists, and it's not available for lists? Unless I'm just missing it when I type in Deck.Contents....it's just not there.. You are right about swap though.. I just thought I would exercise that little ol'brain of mine... considering random_shuffle wasn't there.
Long story short as for that toString function... I tried going about doing it paul's way, and for some reason as for which I do not remember, it just seemed like it didn't "fit" well with me. It might have been a design flaw on my part, which I believe is fixed now, but I don't really care about the speed of it. I'm more concerned as to just get this thing out and working to finally say I've finished something on my own.
I actually just went to the bookstore recently and bought a book on general algorithms and their everyday use in programming. Great book. I know it'll help me down the line on any speed inprovements I might want to take a look at if I get into anything serious.
As for the global objects, I was doubting myself too when it came down to my methods. I'll take your advice mainly because you are able to distinguish it easier.
Thanks again everyone for the great advice!
I still need to learn as to why my iterators are becoming invalidated in certain situations... I still have a problem with my looking for card from another player function
Code:
int CheckFor (const int face, Player& pplayer)
.. I'm using the exact same methods as the draw card function but I run into problems eghh... *sigh*...
For iterator invalidation, but this is generally true when you use the stl:
You MUST know every container, and their functions by heart, and which invalidate what. It is easier if you understand how the container works. If you don't take the time to learn this, you will run into trouble.
As for random shuffle. It did not appear because it is a generic algorithm, like all algorithms in the stl. You are just supposed to call it between begin and end:
It is true that some algorithms were re-written specifically for lists (sort comes to mind). But as a general rule, the algorithms are not member functions of your containers. This is what allows you to call them on anything, even plain old arrays!
For iterator invalidation, but this is generally true when you use the stl:
You MUST know every container, and their functions by heart, and which invalidate what. It is easier if you understand how the container works. If you don't take the time to learn this, you will run into trouble.
As for random shuffle. It did not appear because it is a generic algorithm, like all algorithms in the stl. You are just supposed to call it between begin and end:
It is true that some algorithms were re-written specifically for lists (sort comes to mind). But as a general rule, the algorithms are not member functions of your containers. This is what allows you to call them on anything, even plain old arrays!
UGH, I FORGOT... I feel so dumb. BTW, I JUST figured out my problem... I forgot the erase function.. when you say i = ***.erase(blah), it automatically moves the iterator to the next position... so I just added an else statement and it works perfectly now... ugh >_> THANK YOU SOOO MUCH <3
random_shuffle requires random access iterators. list does not support random access iterators, list iterators are bidirectional. To use random_shuffle the OP would need swop his lists for vectors or deques.
Get Microsoft Visual C++ Express here or CodeBlocks here.
Get STLFilt here to radically improve error messages when using the STL.
Get these two can't live without C++ libraries, BOOST here and Loki here.
Check your code with the Comeau Compiler and FlexeLint for standards compliance and some subtle errors.
Always use [code] code tags [/code] to make code legible and preserve indentation.
Do not ask for help writing destructive software such as viruses, gamehacks, keyloggers and the suchlike.
random_shuffle requires random access iterators. list does not support random access iterators, list iterators are bidirectional. To use random_shuffle the OP would need swop his lists for vectors or deques.
yeah I thought I was going crazy there for a second...
I actually deleted my modified shuffle routine ... and tried random_shuffle again.. to no avail ...
it was like.. NOOOO OMG >__________< *ctrl z ctrl z ctrl z*
random_shuffle requires random access iterators. list does not support random access iterators, list iterators are bidirectional. To use random_shuffle the OP would need swap his lists for vectors or deques.
Strange... Most of the algorithms in the std work with simple forward iterators. They work better with random access, but I thought it would still work with forward iterators.
And looking at the algorithm, I see no reason why it wouldn't work. But it doesn't matter, the standard is the standard, and random_shuffle doesn't work. I stand corrected
@OP, I still recommend you use the algorithm inside the link I gave you. Not only is it much lighter (only N iterations, whereas you do magic number 10000), but it gives you a true random shuffle, i.e., it gives you a uniform distribution on the N! possible permutations.
Strange... Most of the algorithms in the std work with simple forward iterators. They work better with random access, but I thought it would still work with forward iterators.
And looking at the algorithm, I see no reason why it wouldn't work. But it doesn't matter, the standard is the standard, and random_shuffle doesn't work. I stand corrected
It might be an oversight in the standard, but it might also be because we're talking about linear time complexity degrading to quadratic time complexity, if my analysis is correct. Consequently, it might be "dangerous" to allow it to work with mere forward iterators, since it would likely be considerably better to shuffle an auxiliary container with random access.
Speaking of containers with random access... exiledgolem, I am guessing that you chose std::list over std::vector because you want to pop/push front/back, and generally did not need the random access. However, perhaps you should consider std:eque, which would allow you to pop/push front/back in constant time, and also provides random access. Unfortunately, it is not suitable if you need to store, in other objects, iterators to the elements of that container, since std:eque iterators can be invalidated on any insertion or deletion from the std:eque.
C + C++ Compiler: MinGW port of GCC
Build + Version Control System: SCons + Bazaar
* The Best Reasons to Target Windows 8
Learn some of the best reasons why you should seriously consider bringing your Android mobile development expertise to bear on the Windows 8 platform.