Reputation: 229

Overloading operator and modifiyng string

I am learning about operator overlaoding. I have created simple class to test it.

class Beer{
public:
   Beer(int oner , int twor , string name){
   this -> one = oner;
   this -> two = twor;
   this -> name = name;
   };
    int getOne(){

        return this -> one;
    };
    int getTwo(){

        return this -> two;
    };
    string getName(){
        return this -> name;
    };
    Beer operator + (const Beer &a)const {

        return Beer(5,two+a.two,"firstName");

    };
    Beer operator + (string a)const {

       this -> name =  this -> name +" "+a;

    };

private:

   int one;
   int two;
   string name;
};

I am trying to figure out , how to midify the string with overloaded operand. My function i declared

Beer operator + (string a)const {
      this -> name =  this -> name +" "+a;
};

Throws error about passing const string.

I tried using

Beer operator + ( const string *a)const { 
       swap(this -> name , this -> name + " " + a);
       return *this;
    };

Which complained about one being cosnst string , and secon one being basic string.

The idea is simple.

Beer one ( 5, 6, "one")
one + "two"

// one.name = "one two"

What is the right way how to do it?

// error with swap

error: no matching function for call to 'swap(const string&, std::basic_string<char>)'|

// erro with string

passing 'const string {aka const std::basic_string<char>}' as 'this' argument of 'std::basic_string<_CharT, _Traits, _Alloc>& std::basic_string<_CharT, _Traits, _Alloc>::operator=(std::basic_string<_CharT, _Traits, _Alloc>&&) [with _CharT = char; _Traits = std::char_traits<char>; _Alloc = std::allocator<char>]' discards qualifiers [-fpermissive]|

Upvotes: 0

Answers (3)

Kuba hasn't forgotten Monica

Reputation: 98425

Comments:

Don't include the entire std namespace. You're likely to run into nasty name clashes with your own code. At most, use the symbols you need explicitly, e.g. using std::string;.
Unless you need a copy of a value to modify, pass large objects like std::string by const reference. When you declare a parameter as having a value type std::string, you receive a copy of the string, and that's expensive unless you need a copy to modify inside of your function.

This is a long-standing problem with the C++ standard: an implementation detail like this, that should be irrelevant to the user of the function, leaks into the interface (the declaration of the function). Still, when having a copy makes sense, let the compiler give you one without having to type as much. Thus:
```
// prefer this
std::string fooize(std::string foo) {
  assert(foo.size() > 0);
  foo.insert(1, "foo");
  return foo;
}
// over this
std::string fooize(const std::string & bar) {
  assert(bar.size() > 0);
  auto foo = bar;
  foo.insert(1, "foo");
  return foo;
}
```
Use an initializer list, you then won't need to do silly name gymnastics (you had oner, twor names:
```
Beer(int one, int two, const std::string & name) :
  one(one),
  two(two),
  name(name)
{}
```
Declare read-only accessors const:
```
int getOne() const { return one; }
```

Return large values like strings by const reference; the user code will likely have the compiler help out with making a copy when needed automatically:

const std::string & getName() const { return name; }

// use:
Beer beer{0,0,""};
std::cout << (beer.getName() + "!") << std::endl; // makes a copy of name as needed

In the + operator taking a string, you're supposed to return a new object, not modify this. You pretty much should do it the way the other operator + you have did it.
```
Beer operator +(const std::string & a) const {
  return Beer(one, two, name + " " + a);
};
```

If you want to modify your object, you want operator +=:

Beer & operator+=(const std::string & a) {
  name += " ";
  name += a;
  return *this;
}

Even though your class was designed to experiment with operators, you should always consider whether the operators make life easier or not. For example, you class has three members. It's not immediately apparent which of these members would be operated on, unless it was otherwise clear from the class's semantics. It'd be much clearer to have methods named addToOne, addToTwo, and appendToName, for example, instead of operator(s), or simply letting the user set the member through setters, like setOne(int one) { this->one = one; }. The user would then simply do beer.setOne(beer.getOne() + 2);.
Consider naming getters without the get prefix, e.g.
```
class Beer {
  int m_one;
public int one() const { reeturn m_one; }
};
```
It's less typing for the user. The standard library, as well as large libraries like boost and Qt follow this convention, e.g. you have std::string::size(), not std::string::getSize(), etc.

Upvotes: 2

Unimportant

Reputation: 2096

Beer operator + (string a)const {
  this -> name =  this -> name +" "+a;
};

You should not change the contents of the object who's + operator is beeing called, afterall if you perform A = B + C, the contents of B should not change. The compiler is correctly informing you about this because it is a const function.

Rather create a temp object to hold the 'sum' and return it.

Beer operator + (string a)const {
    return Beer(one, two, name + " " + a);  
};

Upvotes: 1

Rob K

Reputation: 8926

In your operator+() here:

Beer operator+( string a ) const 
{
    this->name = this->name + " " + a;
};

the const on the function signature is a guarantee to the compiler that when the function is invoked, you won't change the data in the object, yet you change the data in the object.

Upvotes: 0

Overloading operator and modifiyng string

Answers (3)

Related Questions