I have two classes. One called Agent, and the other called Player. Agent is the base class and Player is the derived class.
When calling the function SetHand() from the derived Player class, I run into a segfault.
#include "deck.hpp"
#include <cstdint>
#include <cstdlib>
#include <memory>
#include <stdexcept>
#include <vector>
#ifndef BLACKJACK_INCLUDE_AGENT_H_
#define BLACKJACK_INCLUDE_AGENT_H_
class Agent {
public:
virtual std::vector<Card> GetHand() { return hand_; }
virtual void UpdateHand() {
if (deck_->empty())
throw std::runtime_error("deck is empty, cannot draw a card");
hand_.push_back(deck_->back());
deck_->pop_back();
}
void SetHand() {
for (uint8_t i = 0; i < 1; i++) {
if (deck_ == NULL)
throw std::runtime_error("deck is empty, cannot draw a card");
hand_.push_back(deck_->back());
deck_->pop_back();
}
}
virtual void UpdateDeck(std::vector<Card> deck) {
deck_ = std::make_shared<std::vector<Card>>(deck);
}
private:
std::shared_ptr<std::vector<Card>> deck_;
std::vector<Card> hand_;
};
#endif // !BLACKJACK_INCLUDE_AGENT_H_
When looking in the debugger, this is because it is trying to use the deck_ in Agent, which is uninitialized.
#include "agent.hpp"
#include "deck.hpp"
#include <memory>
#ifndef BLACKJACK_INCLUDE_PLAYER_H_
#define BLACKJACK_INCLUDE_PLAYER_H_
class Player : public Agent {
public:
Player(std::vector<Card> deck, int initial_chips);
// use negative argument to reduce chip count
void UpdateChips(int chips);
private:
int id_;
bool hand_in_progress = false;
std::shared_ptr<std::vector<Card>> deck_;
std::vector<Card> hand_;
unsigned long chips_;
};
#endif // BLACKJACK_INCLUDE_PLAYER_H_
If I move the function to the Player class then everything works fine. However, I would like for it to be able to inherit this function, as I intend to use it in other classes.
The code where the method is called:
TEST(PlayerDeckTest, InitDeck) {
Deck deck(10);
Player player1(deck.GetDeck(), 100);
player1.SetHand();
std::vector<Card> hand = player1.GetHand();
while(!hand.empty()) {
EXPECT_FALSE(hand.back().GetSuit().empty());
EXPECT_FALSE(hand.back().GetValue() != 0);
hand.pop_back();
}
}
inThe-FLesh is a new contributor to this site. Take care in asking for clarification, commenting, and answering.
Check out our Code of Conduct.
8
Both Agent and Player have their own distinct deck_ and hand_ members. There is no good reason for Player to declare its own, it should instead be using the members that are inherited from Agent:
class Agent {
public:
...
protected: // <-- not private!
std::shared_ptr<std::vector<Card>> deck_;
std::vector<Card> hand_;
};
class Player : public Agent {
public:
...
private:
int id_;
bool hand_in_progress = false;
//std::shared_ptr<std::vector<Card>> deck_; // <- get rid of this!
//std::vector<Card> hand_; // <- get rid of this!
unsigned long chips_;
};
That being said, you should not have each Agent/Player put its std::vector deck inside of a std::shared_ptr. That is just overkill. If you want to use shared_ptr then it should be created inside of the Deck class instead and then handed out to each Agent/Player as needed.
I would suggest changing Agent to store a raw vector* pointer or vector& reference to the original vector object, there is no need to make a separate copy of the vector for each Agent/Player. Each Agent/Player should not be playing cards from its own deck, they should be playing cards from a shared deck:
class Agent {
public:
Agent(std::vector<Card>& deck)
: deck_(deck)
{
}
std::vector<Card> GetHand() { return hand_; }
void DrawCard() {
if (deck_.empty())
throw std::runtime_error("deck is empty, cannot draw a card");
hand_.push_back(deck_.back());
deck_.pop_back();
}
void SetHand() {
hand_.clear();
for (uint8_t i = 0; i < 2; ++i) {
DrawCard();
}
}
protected:
std::vector<Card>& deck_;
std::vector<Card> hand_;
};
class Player : public Agent {
public:
Player(std::vector<Card>& deck, int initial_chips)
: Agent(deck), chips_(initial_chips)
{
}
...
private:
int id_ = 0;
bool hand_in_progress = false;
unsigned long chips_= 0;
};
TEST(PlayerDeckTest, InitDeck) {
Deck deck(10);
std::vector<Card> deckCards = deck.GetDeck();
Player player1(deckCards, 100);
player1.SetHand();
for (auto &card : player1.GetHand()) {
EXPECT_FALSE(card.GetSuit().empty());
EXPECT_FALSE(card.GetValue() != 0);
}
Player player2(deckCards, 10);
player2.SetHand();
for (auto &card : player2.GetHand()) {
EXPECT_FALSE(card.GetSuit().empty());
EXPECT_FALSE(card.GetValue() != 0);
}
}
Even better would be if you have each Agent/Player hold a pointer/reference to the actual Deck object instead of its internal std::vector, eg:
class Deck{
public:
Card DrawCard() {
if (cards_.empty())
throw std::runtime_error("deck is empty, cannot draw a card");
Card card = cards_.back();
cards_.pop_back();
return card;
}
private:
std::vector<Card> cards_;
};
class Agent {
public:
Agent(Deck& deck)
: deck_(deck)
{
}
std::vector<Card> GetHand() { return hand_; }
void DrawCard() {
hand_.push_back(deck_.DrawCard());
}
void SetHand() {
hand_.clear();
for (uint8_t i = 0; i < 2; ++i) {
DrawCard();
}
}
protected:
Deck& deck_;
std::vector<Card> hand_;
};
class Player : public Agent {
public:
Player(Deck& deck, int initial_chips)
: Agent(deck), chips_(initial_chips)
{
}
...
private:
int id_ = 0;
bool hand_in_progress = false;
unsigned long chips_= 0;
};
TEST(PlayerDeckTest, InitDeck) {
Deck deck(10);
Player player1(deck, 100);
player1.SetHand();
for (auto &card : player1.GetHand()) {
EXPECT_FALSE(card.GetSuit().empty());
EXPECT_FALSE(card.GetValue() != 0);
}
Player player2(deck, 10);
player2.SetHand();
for (auto &card : player2.GetHand()) {
EXPECT_FALSE(card.GetSuit().empty());
EXPECT_FALSE(card.GetValue() != 0);
}
}