C++

Forum

 
what's wrong with this function?

technologist (607)
In the following code how do I pass card_objects array through function read_back(card_objects) for use by for (auto elem:card_objects)? I've isolated the error to reside in the function or the for-each in the function itself. Error below.

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
 
#include<iostream>
#include<array>

struct Card_Objects{
	int ranks = 0;
	int suits = 0;
};

void read_back(Card_Objects &card_objects)
{

std::cout<<"Suits"<<"\tRanks"<<std::endl;

for (auto &elem: card_objects)
 {
//std::cout<<elem.suits<<"\t"<<elem.ranks<<std::endl;
//
 }
}

int main()
{
			int ctr(0);
			Card_Objects card_objects[52] = {};
			for (int suits = 1; suits<5;++suits)
			{
			for (int ranks = 2; ranks<15; ++ranks)
			{
			card_objects[ctr].ranks = ranks;
			card_objects[ctr].suits = suits;
			ctr=ctr+1;
			}
			}
			//readback
			read_back(card_objects);

	return 0;
}




..\src\main.cpp:14:18: error: 'begin' was not declared in this scope
 for (auto &elem: card_objects)


.\src\main.cpp:14:18: error: 'end' was not declared in this scope
 for (auto &elem: card_objects)


..\src\main.cpp:35:26: error: invalid initialization of non-const reference of type 'Card_Objects&' from an rvalue of type 'Card_Objects*'
Last edited on
TheIdeasMan (6817)
Hi,

I found an answer here (Googled the error message):

http://stackoverflow.com/questions/28853974/new-c11-for-loop-causes-error-begin-was-not-declared-in-this-scope


1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
 
#include <iostream>
#include <array>

struct Card_Objects {
  int ranks = 0;
  int suits = 0;
};

void read_back(Card_Objects (&card_objects)[52]) {

  std::cout << "Suits"
            << "\tRanks\n";

  for (const auto &elem : card_objects) {
    std::cout << elem.suits << "\t" << elem.ranks << std::endl;
    //
  }
}

int main() {


  int ctr(0);
  Card_Objects card_objects[52] = {};
  for (int suits = 1; suits < 5; ++suits) {
    for (int ranks = 2; ranks < 15; ++ranks) {
      card_objects[ctr].ranks = ranks;
      card_objects[ctr].suits = suits;
      ++ctr;
    }
  }
  // readback
  read_back(card_objects);

  return 0;
}


Note that you should avoid using ordinary arrays, the STL containers are much better. Although you had included the std::array but didn't use it.

The ranged based for loop is easier to use with the STL containers: one doesn't have to resort to the answer shown above.


http://en.cppreference.com/w/cpp/language/range-for


Edit:

Card_objects is perhaps not a good name: it's a type, maybe Card_type or just Card?
In a similar fashion, card_objects could be Deck
Last edited on
technologist (607)
Well you hit the nail on the head. How do you use a range-based loop or container to do what the ordinary loop does above? Its the nesting that throws me off. Once I learn that I can pretty much discard non-container code:

ordinary arrays
1
2
3
4
5
6
7
8
9
10
11
 

Card_Objects card_objects[52] = {};
			for (int suits = 1; suits<5;++suits)
			{
			for (int ranks = 2; ranks<15; ++ranks)
			{
			card_objects[ctr].ranks = ranks;
			card_objects[ctr].suits = suits;
			ctr=ctr+1;
			}
			}


ps. I plan to clean everything up as mentioned above
TheIdeasMan (6817)
How do you use a range-based loop or container to do what the ordinary loop does above?


Not sure whether you realise that the range based for loop is good when you want to iterate through all of the container. So if you had std::array<Card, 52> the range based loop will go through them all as expected.

Otherwise use an ordinary for loop (like you already have), or iterators if you only want to do some of them. You could use iterators to process the cards in groups of 14: one suit at a time.

Its the nesting that throws me off. Once I learn that I can pretty much discard non-container code:


I hope that because you are using a STL container, you are not thinking that means you should / can abandon using for loops to process them ?

There is also the std::generate algorithm, The example in the link below shows how to use a lambda function to produce sequential values.

http://en.cppreference.com/w/cpp/algorithm/generate

IIRC Did someone (Duoas?) mention in a different topic about organising the cards so that the position in the array modulus 4 gives the suit? You might be able to incorporate that into the lambda function.

Can also make use of enums (C++11 has scoped enums) then your cards will have a name. Or, perhaps the Card struct could have a score value as well, with the rank as a std::string.
technologist (607)
I know how to do this->
Or, perhaps the Card struct could have a score value as well, with the rank as a std::string.


Can you give an example of how I would use an enum to name the cards (only a few for examples sake).

The mod suggested to identify suits is very interesting. Another tool in the tool box. So my strategy is to use structs, and possibly enums.
TheIdeasMan (6817)
Hi,

The code below seems to work - Iooked in the debugger, all the Cards seem to be allocated , but I left something for you to have a go at:

The InitDeck function is too long, could split it up so it calls ChooseSuit and ChooseRank functions;
Implement the Print Function. Have a look at this, to overload operator<< using a switch with the enums.

http://en.cppreference.com/w/cpp/language/enum

Could also implement Deck as a class of it's own, with a constructor and the std::list being a private member.

I used std::list instead of std::array, it might be better for shuffling, Dealing etc. Other types like std::unordered_set ,might be OK too.

If you want to convert those enum to an int for a score say, you need to use a static_cast<int>(EnumType::Variable)



1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71
72
73
74
75
76
77
78
79
80
81
82
83
84
85
86
87
88
89
90
91
92
93
94
95
96
97
98
99
100
101
102
103
104
105
106
107
108
109
110
111
112
113
114
115
116
117
118
 
#include <iostream>
//#include <array>
#include <list>

enum class Suit {Hearts, Diamonds, Clubs, Spades};
enum class Rank {Two = 2, Three, Four, Five, Six, Seven, Eight , Nine, Ten,
                Jack, Queen, King, Ace
                };

struct Card {
  Card(const Rank R, const Suit S )
    :
      rank(R),
      suit(S)
  {}

  Rank rank;
  Suit suit;
};

void InitDeck(std::list<Card>& Deck, std::size_t NumCards);
void PrintDeck(std::list<Card>& Deck);

int main() {

  //Card AceClubs(Rank::Ace, Suit::Clubs);

  const std::size_t NumCards =52;


  std::list<Card> Deck;

  //Deck.push_back(AceClubs);
  InitDeck(Deck, NumCards);

  return 0;
}

void InitDeck(std::list<Card>& Deck, std::size_t NumCards) {
    for (std::size_t C = 0 ; C < NumCards ; ++C) {

      const std::size_t CardsPerSuit = 13;

      Suit ThisCardSuit;

      // Choose the suit
      switch (C / 13) { // integer division does what we want here
        case 0:
          ThisCardSuit = Suit::Hearts;
          break;
        case 1:
          ThisCardSuit = Suit::Diamonds;
          break;
        case 2:
          ThisCardSuit = Suit::Clubs;
          break;
        case 3:
          ThisCardSuit = Suit::Spades;
          break;
        default:
          std::cout << "Error in Suit Selection\n";
          break;
      }


      Rank ThisCardRank;

      // Choose the Rank
      switch (C % CardsPerSuit) {
        case 0:
          ThisCardRank = Rank::Two;
          break;
        case 1:
          ThisCardRank = Rank::Three;
          break;
        case 2:
          ThisCardRank = Rank::Four;
          break;
        case 3:
          ThisCardRank = Rank::Five;
          break;
        case 4:
          ThisCardRank = Rank::Six;
          break;
        case 5:
          ThisCardRank = Rank::Seven;
          break;
        case 6:
          ThisCardRank = Rank::Eight;
          break;
        case 7:
          ThisCardRank = Rank::Nine;
          break;
        case 8:
          ThisCardRank = Rank::Ten;
        break;
        case 9:
          ThisCardRank = Rank::Jack;
          break;
        case 10:
          ThisCardRank = Rank::Queen;
          break;
        case 11:
          ThisCardRank = Rank::King;
          break;
        case 12:
          ThisCardRank = Rank::Ace;
        default:
          break;
      }
      Deck.emplace_back(Card(ThisCardRank, ThisCardSuit));
    }

}

void PrintDeck(std::list<Card>& Deck) {

}

TheIdeasMan (6817)
One problem is that we should check that all the cards are always unique, and we want to disallow adding any new cards. This functionality could be included in the Deck class.
technologist (607)
Sorry for the late reply - thank you so much for the enum example. I have a clearer path to achieving some recent goals. The enums provide more readability. Their implementation has not been obvious to me until now.

and we want to disallow adding any new cards.


Perhaps a random function and swap of some sort would prevent this.

technologist
TheIdeasMan (6817)
Another thing, we need to make it so that a Card cannot be changed after it is created: so perhaps you could make Card a class with the data members private, and don't provide any functions to set those values. Edit: Apart form the constructor.

Perhaps a random function and swap of some sort would prevent this.


I was thinking in a Deck class, have a const private variable that is number of cards limit, then an InsertCard function could check to see if that limit had been exceeded. If the default constructor of the Deck class has the code to initialise the Deck I wrote earlier, then this should satisfy the uniqueness criteria as well. Edit2: If it is all in the constructor, don't provide an InsertCard function at all.

There is a std::sort algorithm which could be handy once a Hand has been dealt, it would be nice to have it sorted. It requires an operator< to be defined so that the compiler "knows" how to compare.

Just so you know there is a std::unique algorithm as well, it requires the container be sorted.
Last edited on
technologist (607)
"back to the basics" question. Do I need an iterator to make the function below work?
the printing of the container values is straight forward. What I want to know is whether in "populate_array" whether the array can be populated without iterators by a var (in this case ctr). 
1
2
3
4
5
6
7
8
9
10
11
12
13
14
 
void populate_array(std::array<Card_Type,52>&card_deck)
{

int ctr = 0;
for(int suits = 1; suits<5; ++suits)
{
	for(int ranks = 1; ranks<14; ++ranks)
{
ctr+=1;
card_deck[ctr].suits = suits;
card_deck[ctr].ranks = ranks;
}
}
}

full code:

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
 
#include<iostream>
#include<array>

struct Card_Type
{
	int suits = 0;
	int ranks = 0;
};


void populate_array(std::array<Card_Type,52>&card_deck)
{

int ctr = 0;
for(int suits = 1; suits<5; ++suits)
{
	for(int ranks = 1; ranks<14; ++ranks)
{
ctr+=1;
card_deck[ctr].suits = suits;
card_deck[ctr].ranks = ranks;
}
}
}
void read_back(std::array<Card_Type,52>&card_deck)
{
	std::cout<<"Suit\t"<<"Rank"<<std::endl;
	for(auto & elem : card_deck)
		{ std::cout<<elem.suits<<"\t"<<elem.ranks<<std::endl;}



}
int main()
{
	std::array<Card_Type, 52>card_deck;
	populate_array(card_deck);
	read_back(card_deck);
	return 0;
}


this works. but do I still need the index?
I can provide the other copy not working but in the interest of time and space its not posted.
Last edited on
TheIdeasMan (6817)
What I want to know is whether in "populate_array" whether the array can be populated without indexing by a var (in this case ctr). Will c++ allow me to do this?


Much earlier I mentioned std::generate, the example uses a lambda function. You probably haven't used one of those or iterators yet (I guess). But the part in braces is a function like any other. It is applied to each element in a container, and that is what the iterators do.

http://en.cppreference.com/w/cpp/algorithm/generate

Have a go, start out simple - see what you can do :+)
technologist (607)
awesome
thx
t
Topic archived. No new replies allowed.