0

Can I manipulate string in setter? For example I want to set it length to 20. Is it that possible and is it the best practice? I get "Non const function is called on const object". Code tried:

void setName(const string &name) {
    if (name.size() > 20)
    {
        name.reserve(20);
    }
    Employee::name = name;
}
IntoTheDeep
  • 4,027
  • 15
  • 39
  • 84

4 Answers4

1

No, in your example name is a const string reference so you can only use const methods on it (basically meaning you can't modify name). The reserve is not a const method. The const string& name means that name is the same string as the caller supplied, the const means that you may not modify it.

What you could do however is to create a copy of name somehow. The most obvious way is maybe to create a copy inside the method, this means that you can keep the function signature (which may be a good thing IMHO):

void setName(const string &name) {
    string tname = name;
    if (tname.size() > 20)
    {
        tname.reserve(20);
    }
    Employee::name = tname;
}

Another way you could do it is to pass name by value which means that the argument will be a copy of what was supplied by the caller (one could argue that one shouldn't alter the signature of the method to reflect the implementation, but that's a matter of opinion):

void setName(string name) { // note no ampersand

What you can't/shouldn't do is to only remove the const because that would mean that the argument is the same string as the caller supplied. It will fail to compile if the caller don't supply a mutable name and if supplied a mutable name the callers copy would change as well (since it's the same).

skyking
  • 13,817
  • 1
  • 35
  • 57
  • Of course, I completely misread the sentence or was having a blackout! Sorry, I'm going to delete my comment :) – Christian Hackl Jan 12 '17 at 11:12
  • You should `= std::move(tname)` instead of `= tname` to avoid a needless copy :) (same if you take the param by copy) – Emil Laine Jan 12 '17 at 12:13
  • @tuple_cat My intention was not to make this an optimal solution. The better solution might be to work on `Employee::name` directly and therefore avoiding a needless copy altogether (`move` could still do some needless copying). – skyking Jan 12 '17 at 13:25
1

It seems to me that you are addressing the problem in the wrong way. The code posted calls reserve when the size of the string passed is higher then 20, but that is a non-binding request to shrink the string (which is const, BTW).

If you want to limit that string member of your class to a particular size while passing a const reference in a setter, all you have to do is copying into it only a substring of the passed string.

Bob__
  • 12,361
  • 3
  • 28
  • 42
  • 1
    The `reserve()` call will do nothing here, because `capacity()` must be at least `size()`. – Christian Hackl Jan 12 '17 at 10:36
  • @ChristianHackl Yes, that's actually what I was trying to express with my bad phrasing. – Bob__ Jan 12 '17 at 10:41
  • Now that I'm thinking about it, perhaps my statement is not entirely true either. The string's size may be 30, its capacity 1000. Then `reserve(20)` *may* shrink the capacity to 30. – Christian Hackl Jan 12 '17 at 10:42
  • @ChristianHackl looking at the reference that's the expected behavior, including the _may_... – Bob__ Jan 12 '17 at 10:46
1

I get "Non const function is called on const object"

Of course. That's the whole point of const in the first place!

if (name.size() > 20)
{
    name.reserve(20);
}

This does not make sense at all. It literally says: "If the string has more than 20 characters, then make sure that it can internally hold at least 20 characters".

Now, the call may also have the effect that the string's internal capacity shrinks to whatever size greater than 20 it represents to the outside world. For example, if your string has size 30 and its current capacity is 1000, then reserve(20) may shrink the capacity to 30.

However, this is a low-level memory-management concern a beginner typically doesn't or shouldn't care about. If, what I believe, your intention is merely to cut the string, then you need resize.

I would solve your problem like this, plain and simple:

void setName(std::string const& name)
{
    this->name = name;
    if (this->name.size() > 20)
    {
        this->name.resize(20);
    }
}
Christian Hackl
  • 27,051
  • 3
  • 32
  • 62
0

You cannot mutate const references: this means that you will not be able to call std::string::reserve as it's not const-qualified.

Consider taking name by value:

void setName(string name) 
 {
    if (name.size() > 20)
    {
        name.reserve(20);
    }

    Employee::name = name;
}

In this case, a copy of the string passed to setName will be made, which can be mutated leaving the original unchanged.

Vittorio Romeo
  • 90,666
  • 33
  • 258
  • 416
  • @TeodorKolev: if you need to mutate the argument before setting it, you have to get rid of the `const`. – Vittorio Romeo Jan 12 '17 at 09:27
  • 1
    const helps out caller because he can pass a string literal. – Malcolm McLean Jan 12 '17 at 10:02
  • 1
    I disagree with this approach, because taking the string by value because you need an internal copy means that you have compromised the interface of the function for its implementation. – Christian Hackl Jan 12 '17 at 10:38
  • @Christian Hackl: setters in C++11 should take by value and move anyway. Sink interfaces taking by value are idiomatic – Vittorio Romeo Jan 12 '17 at 11:42
  • @VittorioRomeo: That looked like the new recommendation for a while when C++11 was new, but it's no longer the case. See https://isocpp.org/blog/2014/10/quick-q-why-is-want-speed-pass-by-value-not-recommended-stackoverflow or http://stackoverflow.com/questions/21605579/how-true-is-want-speed-pass-by-value – Christian Hackl Jan 12 '17 at 11:45
  • @ChristianHackl: I know about the drawbacks of the idiom. [I think that this is a great article on the subject](http://blogs.microsoft.co.il/sasha/2014/08/21/c-sink-parameter-passing/). My point is that for `std::string`, when `noexcept` is not involved, and when the string is being retained... pass-by-value + move is superior to pass by `const&`. Regardless, changing the interface of `setName` shouldn't be an issue. – Vittorio Romeo Jan 12 '17 at 11:57
  • @MalcolmMcLean: you can pass a string literal even when passing `std::string` by value. – Vittorio Romeo Jan 12 '17 at 11:58
  • @VittorioRomeo: I know the advantages of the idiom :) But all the arguments in favour of pass-by-value seem to be implementation-centric or optimisation-related. The latter is often not a problem to begin with, and the former disregards the potential for changes. For example, the implementation may evolve into a more complex mechanism where nothing is written to another `std::string` object at all. IOW, it may be a "sink" at the moment, but not in the future. – Christian Hackl Jan 12 '17 at 12:08