2

I'm trying to overload the << operator in this class but this is the output:

Hello, 
Segmentation fault: 11

This is my code:

test.cc:

#include <iostream>
#include "class.h"
#include <string>

using namespace std;

int main() {
    MYString s("Hello");

    MYString s2;

    string hello = "Hello";

    cout << s.text << ", " << s2.text << endl;

    cout << "S: " << s << endl;

    hello[0] = 'M';
    cout << hello << endl;

    return 0;
}

And this is class.h:

#ifndef CLASS_H
#define CLASS_H

#include <string>

using namespace std;

class MYString {
public:
    string text;

    MYString(string data="") {
        text = data;
    }

    friend ostream& operator << (ostream& os, const MYString& data) {
        os << data;
        return(os);
    }
 };


#endif

It compiles fine but I have no idea why it says "Segmentation fault: 11". I have no idea what that means either. Could someone tell me how to fix this? And I'm also really new to C++

(And also I know this code is really pointless but I'm just trying to learn stuff and getting used to C++)

Addison
  • 3,791
  • 3
  • 28
  • 48
  • Avoid using `using namespace std;` see [here](http://stackoverflow.com/questions/1452721/why-is-using-namespace-std-considered-a-bad-practice-in-c) for example on why its bad. And **never** use it in a header file. – AxelOmega Mar 08 '13 at 22:19

5 Answers5

8

You have a stack overflow. Your operator<< calls operator<< (same function with same data).

David Schwartz
  • 179,497
  • 17
  • 214
  • 278
  • 4
    +1: Actually your link should point to your answer. That would be a perfect example for recursion :D. – Zeta Mar 08 '13 at 22:10
  • 1
    I'm surprised that his compiler didn't tag this as infinite recursion. Mine did: "`warning C4717: 'operator<<' : recursive on all control paths, function will cause runtime stack overflow`" – Nik Bougalis Mar 08 '13 at 22:11
  • @NikBougalis: Probably GCC, [as it doesn't provide any warning option for possible recursive calls](http://gcc.gnu.org/onlinedocs/gcc/Warning-Options.html#Warning-Options). – Zeta Mar 08 '13 at 22:18
  • Right - but I'm surprised that `gcc` wouldn't provide this functionality - at least in cases like these, that scream bloody murder. – Nik Bougalis Mar 08 '13 at 22:19
  • There is no need to declare your operator `friend`. First of all your members are all public and secondly a member function has access to the class members. The operator method should also be declared to be const `std::ostream& operator<<(...) const` – AxelOmega Mar 08 '13 at 22:23
  • @Zeta You can certainly define it as a non friend function. – Alex Chamberlain Mar 08 '13 at 22:32
  • @Zeta: I think I was a bit hasty in my comment. I was confused by the function being declared and defined as a friend in the class at the same time. It would be clear to have the `operator<<` function defined outside the class. But it does not need to be a `friend` if all members can be accessed either through getters or by the virtue of them being public. – AxelOmega Mar 08 '13 at 22:34
  • @AxelOmega: True, I was also a bit hasty in my comment, it was a long day. Yes, `friend` is completely unnecessary if the members are public or accessible with getters. – Zeta Mar 08 '13 at 22:35
6
friend ostream& operator << (ostream& os, const MYString& data) {
    os << data; // calls 'ostream& operator<<(ostream&,const MYstring&)' -- oops
    return(os);
}

Infinite recursion. Use os << data.text instead:

friend ostream& operator << (ostream& os, const MYString& data) {
    os << data.text;
    return(os);
}
Zeta
  • 103,620
  • 13
  • 194
  • 236
  • 2
    Make sure you understand *why* this worked and what the difference between your code and the corrected version Zeta showed you is. – Nik Bougalis Mar 08 '13 at 22:12
1

I think you may want to:

os << data.text;

in your overloaded operator, lest you be caught in an infinite recursion. What you currently have will just keep calling itself continuously until your stack blows up.


As an aside, I'm not a big fan of return(os), it makes it look like a function call which it definitely isn't. You can simply do return os, even for very complicated expressions.

And at some point, you will end up having the epiphany that data members should almost always be private, that's what encapsulation is all about. Without that, C++ would just be a harder-to-learn C :-)

paxdiablo
  • 854,327
  • 234
  • 1,573
  • 1,953
0

You want to output the string contained into your MyString object, so you should replace

os << data;

with

os << data.text;

This is the whole point of having a friend function.

As it is, you do an infinite recursion because os << data; calls itself the << operator of your function (data is of type MyString)

Qortex
  • 7,087
  • 3
  • 42
  • 59
0

You should change os << data to os << data.text

SamiHuutoniemi
  • 1,576
  • 2
  • 16
  • 35