CODEWITHSUNDEEP
c++getter-setter
WhyYouNoWork
WhyYouNoWork

Reputation: 293

setter in class won't set variable

I'm currently trying to make a game in C++. In my code I'm trying to nest my variables so that my main doesn't have a lot of includes. My problem right now though is that the value of my variables in my class aren't changing. Stepping through the code it shows it setting the value, but it doesn't work. Anyone know what's going on? Thank you in advance.

This is what I have so far:

Location.h

#ifndef LOCATION_H
#define LOCATION_H

#include <string>

class Location
{
public:
    Location(void);
    Location(std::string name);
    ~Location(void);

    std::string GetName();
    void SetName(std::string value);

private:
    std::string m_Name
};

#endif

Location.cpp

#include "Location.h"

Location::Location(void): m_Name("") {}

Location::Location(std::string name): m_Name(name) {}

Location::~Location(void)
{
}

std::string Location::GetName()
{return m_Name;}

void Location::SetName(std::string value){m_Name = value;}

PlayerStats.h

#ifndef PLAYERSTATS_H
#define PLAYERSTATS_H

#include "Location.h"

class PlayerStats
{
public:
    PlayerStats(void);
    ~PlayerStats(void);

    Location GetLocation();
    void SetLocation(Location location);

private:
    Location m_Location;
};

#endif

PlayerStats.cpp

#include "PlayerStats.h"

PlayerStats::PlayerStats(void): m_Location(Location()) {}

PlayerStats::~PlayerStats(void)
{
}

Location PlayerStats::GetLocation(){return m_Location;}

void PlayerStats::SetLocation(Location location){m_Location = location;}

main.cpp

#include <iostream>
#include "PlayerStats.h"

using namespace std;

PlayerStats playerStats = PlayerStats();

int main()
{
    playerStats.GetLocation().SetName("Test");
    cout<< playerStats.GetLocation().GetName()<<endl;

    return 0;
}

Upvotes: 0

Views: 2664

Answers (2)

Josh Kelley
Josh Kelley

Reputation: 58362

Your immediate issue is that

Location GetLocation();

returns a copy of the location, so when you call SetName here:

playerStats.GetLocation().SetName("Test");

You're changing the name of the temporary copy, and the change is lost as soon as the semicolon is hit.

More broadly, this kind of design (nesting classes and nesting includes so that main doesn't have a lot of includes, and using a.b.c() style code to access nested members) isn't great C++ style:

  • Having a bunch of source files that (transitively) include a bunch of header files means that changing a single header file will trigger recompilations of a bunch of source files. Compile times can be a significant issue in larger C++ projects, so reducing compile times by controlling #include's is important. Read up on "forward declarations" for more information.
  • Writing code like a.b.c() is considered bad object-oriented design, because it reduces encapsulation: not only does the caller have to know about a's details, it has to know about b's also. Sometimes this is the most expedient way to write code, but it's not something to be blindly done just to reduce #include's. Read up on "Law of Demeter" for more information.

Upvotes: 5

bjornruffians
bjornruffians

Reputation: 701

If you want to set the result of playerStats.GetLocation(), you could make GetLocation() pass-by-reference (use ampersand, &, on the return argument). Otherwise you are just setting values in a temporary copy of PlayerStats::m_Location.

Alternatively, you could use the SetLocation() function.

Upvotes: 2

Related Questions