Cannot assign to non-static data member within const member function

Question

I am attempting to use std::unordered_set as a hash table to store many CreditCard's. CreditCard and another class CardDatabase are defined as follows:

class CreditCard {
private:
    string cardHolder;
    unsigned long long cardNumber;
    int limit;
    int balance;

public:
    CreditCard(string in_cardHolder, string in_cardNumber, int in_limit) {
        cardHolder = in_cardHolder;
        cardNumber = stoll(in_cardNumber);
        limit = in_limit;
        balance = 0;
    }

    void ChangeBalance(int amount) const {
        balance += amount; // SECOND ERROR
    }
};

class CardDatabase {
private:
    unordered_set<CreditCard> cards;
    unordered_set<CreditCard>::iterator iter;

public:
    CardDatabase() { }
    
    void AddCard(cardHolder, cardNumber, int limit) {
        CreditCard tempCard = CreditCard(cardHolder, cardNumber, limit);
        cards.insert(tempCard);
    }

    void Charge(string cardHolder, int chargeAmount) {
        iter = cards.find(cardHolder);
        iter->ChangeBalance(chargeAmount); // FIRST ERROR
    }
}

Initially I was getting the following compile error at FIRST ERROR: Member function 'ChangeBalance' not viable: 'this' argument has type 'const CreditCard', but function is not marked const. So, I added the "const" to the ChangeBalance function. However, after doing that I get the following compile error at SECOND ERROR: Cannot assign to non-static member within const member function 'ChangeBalance'.

Is there any way to fix this error without changing balance to a static variable? It is obviously important that the balance be different for each CreditCard instance.

Any help is appreciated.

EDIT:

Thank you all for your quick answers. I feel I should clarify something. I already added the proper hash functionality elsewhere in my code:

namespace std {
    template <>
    struct hash<CreditCard> {
        size_t operator()(const CreditCard& cc) const
        {
            return hash<string>()(cc.GetCardHolder());
        }
    }
}

Also, the code I posted initially pasted is from a much larger code base and I didn't delete all the necessary namespacing stuff at first before posting the question. My apologies for the confusion.

What do you expect `CreditCard::CreditCard` and `CreditCard::ChangeBalance` to do? — tkausl, Oct 06 '16 at 02:04

Sam Varshavchik · Accepted Answer · 2016-10-06T02:37:30.073

Members of an unordered_set are constant, and cannot be changed once they're in the unordered_set, by default. You are trying to change the objects in the set, and the compiler is properly telling you that you can't do this.

The only possible way to do this correctly (explained only for educational purposes, because this is bad class design):

Explicitly declare the individual fields that can be modified in this manner as mutable.
Use a custom hash function with your unordered_set, and the hash function must exclude the value of mutable fields from the value of the calculated hash.

Otherwise, modifying the contents of the object in the set obviously changes its hash value, which will result in undefined behavior.

Again, this is explained for informational purposes only. This is not a good class design.

The clean way to do this would be to assign a unique identifier to each CreditCard (you know, like a credit card number?), and use an ordinary std::map, to look up CreditCards by their number.

Thanks for your answer and your helpful explanation – bpgeck Oct 06 '16 at 02:42 — bpgeck, Oct 06 '16 at 02:42

score 2 · Answer 2 · answered Oct 06 '16 at 02:06

2

It's not appropriate for ChangeBalance to have const semantics. By the very nature of it's name, you're modifying the object. Make the function non-const.

void ChangeBalance(int amount) {
    balance += amount;
}

The other problem is that you didn't call your function correctly. You should instead do this:

iter->ChangeBalance(chargeAmount);

I will mention there are cases where you want to modify values in a const object, and there is a mutable type modifier for that. Do not use it to solve your current error, however!

answered Oct 06 '16 at 02:06

paddy

60,864
6
61
103

Thanks for responding so quickly. The reason for the `CreditCard::` was because I was copying only the parts where I have errors from a much larger code base. I have edited my question accordingly – bpgeck Oct 06 '16 at 02:29
`iter->CreditCard::ChangeBalance(chargeAmount);` is correct , the `CreditCard::` is redundant in this case (but there would be other class hierarchies where you might want to include this). – M.M Oct 06 '16 at 02:39

score 1 · Answer 3 · answered Oct 06 '16 at 02:08

void ChangeBalance(int amount) should not be const - it is changing the object.

The problem is before in the iterator: cards.find returns a const object, so you are not allowed to change it.

A way to resolve that is to make your cards set a set of pointers to cards, not of cards; or to use another way to find the matching card

score 0 · Answer 4 · edited May 23 '17 at 12:18

Playing fast and loose with the C++ syntax in that thar code, Hoss. Plenty of errors wait around the corner

First Error:

iter->CreditCard::ChangeBalance(chargeAmount);

should be

iter->ChangeBalance(chargeAmount);

Straight-up bad syntax that likely results from flailing around because of the errors resulting from unordered_set having no idea how to hash a CreditCard. Give this a read: How do I use unordered_set? That said, unordered_set is probably not the right solution for this job. std::map<std::string, CreditCard> looks more on point.

Using the wrong solution to fix the above problem lead to the

Second Error:

void ChangeBalance(int amount) const

const on a method means the method cannot change the state of the object. in ChangeBalance balance += amount; attempts to change the state of the object by updating a member variable.

In addition, the compiler is going to HATE the CreditCard:: in this:

CreditCard::CreditCard(string in_cardHolder, string in_cardNumber, int in_limit) {
    cardHolder = in_cardHolder;
    cardNumber = stoll(in_cardNumber);
    limit = in_limit;
    balance = 0;
}

score 0 · Answer 5 · answered Dec 30 '21 at 10:19

Another solution is to make the "balance" as a static member.

    class CreditCard {
private:
    string cardHolder;
    unsigned long long cardNumber;
    int limit;
    static int balance;
    ....
   }

And then initialize it in cpp file

int CreditCard::balance = 0;

This code may not be very secure. But this can be one of the workaround.

Cannot assign to non-static data member within const member function

EDIT:

5 Answers5