CODEWITHSUNDEEP
c++setter
Jozef
Jozef

Reputation: 75

C++ setter doesn't change variable value

I've class Restaurant containing map of Order. Every order has a number. I need to maintain if an order isn't paid or already is. I've got variable isPaid in Order class, it's false defaultly.

The method Order::pay() should change it to true. It's kind of setter. However, it doesn't work. In main you can see, it returns 0in both cases, however, in second case should return 1 because it's already paid. Maybe there's possibility to call the method from Restaurant class, however, I'd prefer to call it from Order class by something like: restaurant.getOrder(1).pay()

#include <iostream>
#include <map>
#include <iomanip>
#include <string>

using namespace std;

class Order {
    bool isPaid = false;

public:
    bool getIsPaid() {
        return isPaid;
    }

    void pay() {
        isPaid = true;
    }
};

class Restaurant {
    map<int, Order> allOrders;

public:
    void addOrder(int number) {
        Order order = Order();
        allOrders.insert(pair<int, Order>(number, order));
    }

    Order getOrder(int number) {
        return allOrders[number];
    }
};

int main() {
    Restaurant restaurant;
    restaurant.addOrder(1);
    cout << restaurant.getOrder(1).getIsPaid() << endl;
    restaurant.getOrder(1).pay();
    cout << restaurant.getOrder(1).getIsPaid() << endl;
}

Result:

0
0

Upvotes: 0

Views: 1261

Answers (2)

lubgr
lubgr

Reputation: 38287

The member function

Order getOrder(int number);

returns the object by value. When you "pay" as in

restaurant.getOrder(1).pay();
//         ^^^^^^^^^^^ returns a copy

you operate on a temporary copy of the order instance. You can fix this as suggested by @MatthieuBrucher in the comments by e.g. changing the function signature to

Order& getOrder(int number);

Upvotes: 3

Fantastic Mr Fox
Fantastic Mr Fox

Reputation: 33864

In this line:

restaurant.getOrder(1).pay();

the restaurant.getOrder(1) returns a copy of the order, you set the bool in that copy. Consider redesigning your class, returning references to your internals is not always a good idea. You might be better off with something like this:

class Restaurant {
    ...
    public: void processPayment(int orderNumber) {
        auto order = allOrders.find(orderNumber);
        if (order == std::end(allOrders)) { throw std::invalid_argument(); }
        order->second.pay();
    }
    ...
};

Upvotes: 2

Related Questions