My new GoFish thread.

[monarch_dodra]

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.

Could be, could be. I always thought the philosophy behind the std was "we give you the best tools we can, but it is your responsability to use them right".

To get back to OP, though, why do you need to popFront? Your cardStack (I'm avoiding the word deque) can only be read from one "tip" right? Why don't you just use back() and pop_back()? That would solve a bunch of your problems. Also, you will be able to use a vector, and random_suffle.

[exiledgolem]

Could be, could be. I always thought the philosophy behind the std was "we give you the best tools we can, but it is your responsability to use them right".

To get back to OP, though, why do you need to popFront? Your cardStack (I'm avoiding the word deque) can only be read from one "tip" right? Why don't you just use back() and pop_back()? That would solve a bunch of your problems. Also, you will be able to use a vector, and random_suffle.

It was originally done using popback and I only found out I made more problems by doing with popfront etc, so I changed it back last night. You're right about the vector, but I could only use a vector for the deck, not the hand consider the check for cards function from the hand removes a card from ANY place thus invalidating the iterator.. I already went through that , so I'll just keep a list for both.

[exiledgolem]

Well everyone.. I'm FINALLY FINALLY FINALLLyyyyy finished with the whole program.

There are a lot of improvements I'm going to make to it.
But the program is finally functional and error prone.

A couple quick things though...

You may choose ANY card to ask for, even if you don't have it in your hand...
I plan to change this asap, to where you can't, because I believe that is the "norm?"

I only support 2 players and 1 deck right now..
I plan to eventually do it for 8 players / 4 decks..

Gofish currently doesn't show the sets of cards that have been removed
from the game. Shouldn't be too hard to add in.

I also want to add which card was drawn from the Deck..
string style...

add some timers for the highscores.. so the time in minutes / seconds + the number of sets taken... output it to a file... well of course encrypt the information first...

add a name to the player class. Add some options for decks / players / maybe a couple
rules. Add some credits for all who have helped )) if you'd like.

My longterm goal was actually to change this whole concept over to a basic console
input / output with graphics with the SDL library (which I FINALLY found out how to link it using my compiler Visual Studio 2008 prof).

I am still a little iffy on the program control in the main loop...

but thanks again all! Any questions, comments, or concerns... pleeeaseeee post them.

[Paul McKenzie]

You still need to realize when you're repeating code, or when the code has an obvious pattern to it. This code could be half the size it is now if you do not repeat code blocks that only differ by a constant or a variable.

The sign of a good programmer is one that recognizes repeated code, and rewrites it so that the code does not repeat. For example:

void Card::toString()		
{
  //...
   static const char* FaceOuput[] = {"Ace", "Two", "Three", "Four" /*etc */};
   if ( Face >=1 && Face <= 12 )
       cout << FaceOutput[Face - 1];
   else
        cout << "ERROR";
}

That gets rid of over half of your toString() function.

Regards,

Paul McKenzie

[exiledgolem]

You still need to realize when you're repeating code, or when the code has an obvious pattern to it. This code could be half the size it is now if you do not repeat code blocks that only differ by a constant or a variable.

The sign of a good programmer is one that recognizes repeated code, and rewrites it so that the code does not repeat. For example:

void Card::toString()		
{
  //...
   static const char* FaceOuput[] = {"Ace", "Two", "Three", "Four" /*etc */};
   if ( Face >=1 && Face <= 12 )
       cout << FaceOutput[Face - 1];
   else
        cout << "ERROR";
}

That gets rid of over half of your toString() function.

Regards,

Paul McKenzie

Yeah, I implemented it. It helped a lot. Thanks for the advice. I've come into another problem though. Streams.. gosh those streams.. just getting correct input.. I've finally got it down.. but I want to limit the cards the player can choose to just those in their own hand.. so if they only have 5's, 7's, and 8's they can only ask for those cards..

do
{
     // a lot of cout stuff
     cin >> myChoice // ask for which card they want to grab from the other player
     
     if (!cin)
     {
          cin.clear();
          cin.ignore(numeric_limits<streamsize>::max(), '\n');
     }

} while (myChoice > 13 || myChoice < 1 || (Me.badInput()==true));

bool Player::badInput()
{
	list <int> temp;

	list <Card>::iterator i;
	list <int>::iterator b;

	i = this->Hand.begin();

	for (i; i != Hand.end(); ++i)
	{
		temp.push_back(i->Face);  
	}

	b = temp.begin();

	for (b; b != temp.end(); )
	{
		if (!((*b) ==  myChoice))
		{
			cout << "\n\n\tYou can't choose a card you don't have.\n\t";
			system ("pause");
			system ("cls");
			return true;
		}

		else
		++b;
	}

	return false;
}

I believe my algorithm is correct to see if they are asking for a card that isn't in their hand.
The function returns true, but yet, in the program... it keeps sticking saying that they are asking for a card they don't have,
100 percent of the time, no matter what they input.

I don't know what I'm doing wrong. I've been at this one for 3 + hours now using google. The function asks for the input.
I've tried it a couple different ways, but nothing right yet.

[Paul McKenzie]

Yeah, I implemented it. It helped a lot. Thanks for the advice. I've come into another problem though. Streams.. gosh those streams.. just getting correct input..

Worry about the algorithm and structure of your program first before you talk about the user interface.

Anyway, input should be separated from the actual program engine. If you decide to change this to a GUI program, and if you tightly integrate the input with the GoFish logic, you will have a hard time separating or breaking down your program.

I've finally got it down.. but I want to limit the cards the player can choose to just those in their own hand.. so if they only have 5's, 7's, and 8's they can only ask for those cards..

Write a function that returns the valid cards a user can ask for, given what they have. That is the only thing you should do, and again, not try to tightly couple I/O with this. You'll just get yourself in a mess if you mix up I/O with the GoFish logic.

Regards,

Paul McKenzie

[exiledgolem]

Worry about the algorithm and structure of your program first before you talk about the user interface.

Anyway, input should be separated from the actual program engine. If you decide to change this to a GUI program, and if you tightly integrate the input with the GoFish logic, you will have a hard time separating or breaking down your program.
Write a function that returns the valid cards a user can ask for, given what they have. That is the only thing you should do, and again, not try to tightly couple I/O with this. You'll just get yourself in a mess if you mix up I/O with the GoFish logic.

Regards,

Paul McKenzie

Thank you for the great advice. I was thinking this, and it's not REALLY tightly integrated with it. I actually plan to move all the classes to header's and make just an input header to seperate everything (including removing cout in the main logic classes' functions). I was only doing it as a short-term thing. I wanted to finish a project for once, but I definately will take your advice and seperate everything pretty soon. It's still a mess for me. I thought I couldn't return more then one object for a value though? I'll look into that one.. because that was my first though... maybe return a vector / list... but returning mutiple card objects would be EVEN better.

[Paul McKenzie]

because that was my first though... maybe return a vector / list... but returning mutiple card objects would be EVEN better.

You would return some sort of container (vector, list, etc.) of the cards that can be chosen. This function shouldn't get involved in any I/O at all. The function shouldn't know whether you are writing a simple console program, GUI program, etc... It shouldn't even know what part of the game is being executed. All it knows that it has a list of cards, and given this list, return another list that shows what cards can be chosen.

The caller to this function can do whatever it wants with the returned list of cards.

Regards,

Paul McKenzie

[exiledgolem]

Actually Paul I was able to fix my problem without the use of returning a container (thankfully).
I reread my logic a million times and I guess I was just going about it wrong. I finally came up with this idea.

bool Player::badInput(const int input)		// returns true if the value set as a parameter do not match any card in their hand
{											// example, Me.badInput(3) .. 3 is a three of any kind.. if I'm asking for a 3 and it's not in my hand
											// the function returns true, and false if my choice is equal to ANY one of my cards in my hand.
	
	int count = 0;							// used to count if ANY card is equal to my input

	list <Card>::iterator i;
	i = Hand.begin();

	for (i; i != Hand.end(); ++i)			// go through the whole hand.
	{
		if (input == (*i).Face)				// if the input is equal to the face value of that current card
			++count;						// increase the count
	}

	if (count > 0)							// if we got a match with one of cards in our hand with our input
		return false;						// we got good input
	else
		return true;						// if no cards matched... it's bad input, returns true

}

I started to work on the highscores and the encryption now.. and what a PAIN..
hahah <3 thanks again though everyone.

[Joeman]

Just a simple ideal would be to change this

    
if (count > 0)      
     return false; 
else
    return true;

to

 
    return count == 0;

[Paul McKenzie]

Actually Paul I was able to fix my problem without the use of returning a container (thankfully).

Here is the same code:

bool Player::badInput(int input)
{
   for (list <Card>::iterator i = Hand.begin(); i != Hand.end(); ++i)		
   {
	if (input == (*i).Face)
            return false;
   }
   return true;
}

OR

#include <algorithm>
//...
struct InputChecker
{
   InputChecker(int input) : m_input(input) { }

   bool operator() (const Card& card) const
   { return card.Face == m_input; }

   int m_input;
};

bool Player::badInput(int input)
{
   InputChecker(input) IC;
   return std::find_if( Hand.begin(), Hand.end(), IC) ==  Hand.end());
}

Regards,

Paul McKenzie