1

I am currently coding my own big project for a Uni assignment. I have nearly completed most of it but I am currently stuck on an error that I am not sure how to fix. Every time I compile my code using g++ and run the resulting program I am given the error "Segmentation Fault (core dumped)".

I was told by a friend that it may be due to my player::addItem class is using a string, where it should be a void. However, if I change that to void do I also need to change the function in the main to void? Will this fix the error?

This is my player.cpp file:

#include "Player.h"
#include <iostream>
#include <string>
using namespace std;

Player::Player(){

}

void Player::setName(std::string myName){
    name = myName;
}

//Get the players name
std::string Player::GetName()
{
    return name;
}

//Add item to players inventory
std::string Player::AddItem(std::string item, int itemPrice)
{
    //Is players inventory not full?
    if (IsInventoryFull())
    {
        std::cout << "Inventory is full.";
    }

    else
    {
        //Can player afford item?
        if (goldCoins >= itemPrice)
        {
            goldCoins -= itemPrice;
            numbOfItems++;
            std::cout << "You have purchased " << item << "." << "\n";
            inventory.push_back(item); //Add item to inventory
            return item;
        }

        //If player cant afford item 
        else
        {
            std::cout << "You cannot afford this item." << "\n";
        }
    }
}

void Player::SellItem(int itemNum, int itemPrice)
{
    char response;
    std::cout << "Are you sure you want to sell: " << inventory[itemNum] << "? 'y' - Yes. 'n' - No." << "\n";
    std::cin >> response;

    switch (response)
    {
    case 'y':
        numbOfItems++;
        goldCoins += itemPrice;
        inventory.erase(inventory.begin() + itemNum);
        break;

    case 'n':
        std::cout << "That is ok." << "\n"; 
        break;

    default:
        std::cout << "Please enter correct data." << "\n";
    }
}




//Check to see if players inventory is full
bool Player::IsInventoryFull()
{
    //If players inventory isnt full
    if (numbOfItems < maxNumbItems)
    {
        return false;
    }

    //If players inventory is full
    else
    {
        return true;
    }
}

//Return size of players inventory
int Player::InventoryCapacity()
{
    return inventory.size();
}

//Get the players gold
int Player::GetGold()
{
    return goldCoins;
}

//List the players inventory
void Player::ListInventory()
{
    int itemNumb = 0; //item number in menu

    for (int i = 0; i < inventory.size(); i++)
    {
        itemNumb++;
        std::cout << itemNumb << ": " << inventory[i] << "\n";
    }

}

int Player::GetNumbOfItems()
{
    return numbOfItems;
}

Player::~Player()
{
}

This is my main function (shop.cpp):

// Shop.cpp : Defines the entry point for the terminal (e.g. main)


#include "Player.h"
#include "ShopKeeper.h"
#include <iostream>
#include <string>
using namespace std;

int main()
{
    const int maxNumbItems = 5; //Maximum number of items that inventory can store
    int goldCoins = 150, //Amount of gold coins the player has
    numbOfItems = 0; //Number of con-current items player holds
    std::vector<std::string> inventory; //Players inventory
    std::string name = "Gorrex"; //Players name
    Player* player; //The player
    player->setName(name);
    ShopKeeper shopKeeper; //The shop keeper
    int response; //Menu navigation
    std::cout << "Greetings " << player->GetName() << ". Feel free to browse our exotic collectin of fruits." << "\n";
    std::cout << "1: Purchase Items. 2: Sell Items. 3: List Your Items. 4: Show Gold. 5: Exit" << "\n";

    do
    {
        std::cin >> response;

        switch (response)
        {
        case 1:
            shopKeeper.PurchaseItem(player);
            break;

        case 2:
            shopKeeper.SellItem(player);
            break;

        case 3:
            player->ListInventory();
            break;

        case 4:
            std::cout << "You have " << player->GetGold() << " gold coins." << "\n";
            break;

        case 5:
            std::cout << "Thank you for shopping." << "\n";
            break;

        default:
            std::cout << "Please enter valid data." << "\n";
            std::cout << "1: Purchase Items. 2: Sell Items. 3: List Your Items. 4: Show Gold. 5: Exit" << "\n";
        }

        std::cout << "1: Purchase Items. 2: Sell Items. 3: List Your Items. 4: Show Gold. 5: Exit" << "\n";

    } while (response != 5);


    //Keep window open
    std::string barn;
    std::cin >> barn;
    return 0;
}

Should I make a void IsInventoryFull variable and call that instead of the string version of "Inventory is full"?

Code that uses pointers:

#include "ShopKeeper.h"
#include "Player.h"
#include <iostream>
#include <string>
using namespace std;

//Player purchases item from shop keeper
void ShopKeeper::PurchaseItem(Player* player)
{
    //Player player;

    int response = 0; //Menu navigation
    std::cout << "1: Orange - 30 gold. 2: Apple - 50 gold. 3: Lemon - 10 gold. 4: Dragon Fruit - 75 gold. 5: Mandarin - 25 gold." << "\n";

    do
    {
        std::cin >> response;

        switch (response)
        {
        case 1:
            player->AddItem("Orange", 30);
            break;

        case 2:
            player->AddItem("Apple", 50);
            break;

        case 3:
            player->AddItem("Lemon", 10);
            break;

        case 4:
            player->AddItem("Dragon Fruit", 75);
            break;

        case 5:
            player->AddItem("Mandarin", 25);
            break;

        default:
            std::cout << "Please enter valid data." << "\n";
            std::cout << "1: Orange - 30 gold. 2: Apple - 50 gold. 3: Lemon - 10 gold. 4: Dragon Fruit - 75 gold. 5: Mandarin - 25 gold." << "\n";
        }
    } while (response > 5 || response < 1);

}

//Shop  keeper sells item to player
void ShopKeeper::SellItem(Player* player)
{
    //Player player;
    int response = 0;
    player->ListInventory();
    if (response < player->GetNumbOfItems())
    {
        std::cin >> response;

        switch (response)
        {
        case 1:
            player->SellItem(0, 20);
            break;

        case 2:
            player->SellItem(1, 20);
            break;

        case 3:
            player->SellItem(2, 20);
            break;

        case 4:
            player->SellItem(3, 20);
            break;

        case 5:
            player->SellItem(4, 20);
            break;

        default:
            std::cout << "Please enter valid data." << "\n";
            player->ListInventory();
        }
    }

    else
    {
        std::cout << "Item doesn't exist."; 
    }

}

ShopKeeper::ShopKeeper()
{
}


ShopKeeper::~ShopKeeper()
{
}
halfer
  • 19,824
  • 17
  • 99
  • 186
  • Are you sure you're not running your program and *then* you're getting the segmentation fault error? If you're running your program, that means your program has bugs -- it has nothing to do with the compiler. – PaulMcKenzie May 20 '18 at 01:46
  • I'm not entirely sure what you mean by that? When I compile my .cpp files using the terminal and "g++ *.cpp -o count", this works. But then when I use "./count" into terminal, I get the segmentation error. – Alexander Moshos May 20 '18 at 01:48
  • `Player* player; player->setName(name);` -- You are using a pointer that is not initialized. Something like this would give a segmentation fault when you run your program. Also, if your compiler is really broken, *don't use it -- fix it or reinstall it*. You cannot rely on something that is responsible for building your program to be broken. – PaulMcKenzie May 20 '18 at 01:51
  • 4
    So the error has nothing at all to do with the compiler. The compiler built your code, and now you're running your program. Like I said, your program has bugs, and one of them is the one I pointed out in my previous comment. Your friend had you chase wild geese. – PaulMcKenzie May 20 '18 at 01:54
  • 1
    `Player player; player.setName(name);` -- No need for pointers. – PaulMcKenzie May 20 '18 at 02:01
  • Well, looks like we aren't friends anymore. I was under the impression that pointer has been initialised..what is wrong with it? How would I initialise it then? Thanks for the help – Alexander Moshos May 20 '18 at 02:03
  • Friend actually did mark the location of a bug, even if they got the bug wrong. All paths through a non-`void` function must return a value. `AddItem` doesn't. return on all paths. – user4581301 May 20 '18 at 02:05
  • @AlexanderMoshos -- There is no need for a pointer. Just declare a non-pointer variable. – PaulMcKenzie May 20 '18 at 02:06
  • Fisrt thing to ask yourself when you encounter a pointer is, "Does this need to be a pointer?" – user4581301 May 20 '18 at 02:06
  • Okay well first issue; if I dont use pointers then my other code wont work. Should i remove all as pointers or? This is the other code: – Alexander Moshos May 20 '18 at 02:07
  • Handy reading: https://stackoverflow.com/questions/22146094/why-should-i-use-a-pointer-rather-than-the-object-itself – user4581301 May 20 '18 at 02:07
  • @AlexanderMoshos What do you mean "your code won't work"? In `main`, declare a non-pointer, and then just pass the address of that variable. – PaulMcKenzie May 20 '18 at 02:08
  • Second issue. How do I return a value to AddItem while its in the form of std::string and not void? – Alexander Moshos May 20 '18 at 02:09
  • @AlexanderMoshos -- You wrote the function to return a string, thus it is part of your design. What were you told to return if the item could not be added? You or your teacher is the best one to answer that. If you're not supposed to return a value at all, the function should be `void`. – PaulMcKenzie May 20 '18 at 02:12
  • `Player player;` then `player.setName(name);` and even later `shopKeeper.PurchaseItem(&player);`, but instead prefer to use references: `void ShopKeeper::PurchaseItem(Player& player)` and then `shopKeeper.PurchaseItem(player);`. Some reading on references: http://en.cppreference.com/w/cpp/language/reference – user4581301 May 20 '18 at 02:12
  • So I should be using ShopKeeper::PurchaseItem(Player& player); , or should i be using shopKeeper.PurchaseItem(player); ? – Alexander Moshos May 20 '18 at 02:15
  • @PaulMcKenzie I want to return the string "Inventory is full", hence why I used if (IsInventoryFull()) { std::cout << "Inventory is full."; } Sorry im bad at formatting on stackoverflow first time using it lol – Alexander Moshos May 20 '18 at 02:17
  • `ShopKeeper::PurchaseItem(Player& player)` declares a function. `shopKeeper.PurchaseItem(player);` calls it. [The first few chapters of a good C++ text](https://stackoverflow.com/questions/388242/the-definitive-c-book-guide-and-list) goes over all this in detail. – user4581301 May 20 '18 at 02:21
  • Hmm, okay I've got that down packed. Program seems to be compiling now however each time I access a part of the menu some errors will show. Will try figure it out...Thanks for all the help guys appreciate it – Alexander Moshos May 20 '18 at 02:30

0 Answers0