0

I am trying to make a program to simulate playing Battleship in C++

#include <iostream>
#include <vector>

class Ship {
public:
    Ship();
    Ship operator<<(const Ship&);
private:
    int x = 0;
    int y = 0;
    std::vector<std::pair<int, int>> ship_loc;      //Ship location
};

Ship::Ship() {
    srand(time(NULL));
    std::vector<std::pair<int, int>> ship_loc;      //Ship location
    for (int i = 0; i < 4; ++i) {
            x = rand() % 20;
            y = rand() % 20;
            std::pair<int, int> coordinates = std::make_pair(x, y);
            ship_loc.push_back(coordinates);
            //ship_loc.push_back(std::pair<x, y>)
    };
};

Ship operator<<(const Ship &s) {
    std::cout << ship_loc[0] << ship_loc[1] << ship_loc[2] << ship_loc[3]
            << std::endl;
}

int main()
{
    Ship first_ship;
    std::cout << first_ship;
}

Whenever I try to compile this it give me:

battleship.cpp:26:30: error: âShip operator<<(const Ship&)â must take exactly two           arguments
battleship.cpp: In function âint main()â:
battleship.cpp:34:25: error: cannot bind âstd::ostream {aka std::basic_ostream<char>}â     lvalue to âstd::basic_ostream<char>&&â
In file included from /usr/include/c++/4.7/iostream:40:0,
             from battleship.cpp:1:
/usr/include/c++/4.7/ostream:600:5: error:   initializing argument 1 of     âstd::basic_ostream<_CharT, _Traits>& std::operator<<(std::basic_ostream<_CharT, _Traits>&&,     const _Tp&) [with _CharT = char; _Traits = std::char_traits<char>; _Tp = Ship]â

Obviously not very experienced with classes yet. At all.

UnworthyToast
  • 825
  • 5
  • 11
  • 21

3 Answers3

4

The following line does not do what you intend to be:

  Ship operator<<(const Ship&);

You cannot return Ship and the first parameter has to be of ostream type when you overload the << operator. Its return type has to be std::ostream&.Quoting from one of the answers from this thread: C++ Operator Overloading

The stream operators, among the most commonly overloaded operators, are binary infix operators for which the syntax specifies no restriction on whether they should be members or non-members. Since they change their left argument (they alter the stream’s state), they should, according to the rules of thumb, be implemented as members of their left operand’s type. However, their left operands are streams from the standard library, and while most of the stream output and input operators defined by the standard library are indeed defined as members of the stream classes, when you implement output and input operations for your own types, you cannot change the standard library’s stream types. That’s why you need to implement these operators for your own types as non-member functions.

One way is to implement it with the canonical form:

std::ostream& operator<<(std::ostream& os, const Ship& obj)
{
    // write obj information
    return os;
 }
Community
  • 1
  • 1
taocp
  • 23,276
  • 10
  • 49
  • 62
  • And how would I get my overloaded operator to be able to use ship_loc? I was thinking that because I declared ship_loc in the class before defining it in the constructor that it would be able to be used by anything in the class, but apparently not. – UnworthyToast Mar 21 '14 at 23:39
  • @UnworthyToast: `ship_loc` is a member of `Ship`, so you can use taocp's function and say `obj.ship_loc[0]` etc. – AndyG Mar 21 '14 at 23:46
  • @AndyG when I do `std::cout << first_ship << std::endl;` using the method you described in the operator, it gives me: `error: cannot bind âstd::ostream {aka std::basic_ostream}â lvalue to âstd::basic_ostream&&â In file included from /usr/include/c++/4.7/iostream:40:0, from battleship.cpp:1: /usr/include/c++/4.7/ostream:600:5: error: initializing argument 1 of âstd::basic_ostream<_CharT, _Traits>& std::operator<<(std::basic_ostream<_CharT, _Traits>&&, const _Tp&) [with _CharT = char; _Traits = std::char_traits; _Tp = std::pair]â` – UnworthyToast Mar 21 '14 at 23:53
  • @UnworthyToast: Sorry, I didn't realize that `ship_loc[0]` was type `pair`, so you must say `ship_loc[0].first << ", " << ship_loc[0].second` – AndyG Mar 22 '14 at 00:15
  • "The error is here" - that is not an error, it overloads the `<<` operator to take `Ship` as both operands and return `Ship`. That's fine. Operator `<<` existed for a long time before anyone thought of iostreams. His errors come later in the file. (He certainly didn't intend to do this but it is legal nonetheless). – M.M Mar 22 '14 at 00:33
4

You declared operator << as a member function

Ship operator<<(const Ship&);

It means that the left operand is an instance of class Ship. So it could be called as

Ship a, b;

a << b;

But it is obvious that you did not want this.

if you want to output an object of class Ship in output stream then the operator has to be a non-member function. You could define it as a friend function of the class. for example

class Ship {
public:
    Ship();
    friend std::ostream & operator <<( std::ostream &os, const Ship& );
private:
    int x = 0;
    int y = 0;
    std::vector<std::pair<int, int>> ship_loc;      //Ship location
};

And then you could define it as for example the following way

std::ostream & operator <<( std::ostream &os, const Ship &s) 
{
    os << "( " << s.ship_loc[0].first << ", " << s.ship_loc[0].second << " )" 
       << ", ( " << s.ship_loc[1].first << ", " << s.ship_loc[1].second << " )"
       << ", ( " << <.ship_loc[2].first << ", " << s.ship_loc[3].second << " ) "
       << ", ( " << <.ship_loc[3].first << ", " << s.ship_loc[3].second << " ) "
       << std::endl;

   return os;
}

Also as the number of coordinates is fixed then there is no need to use std::vector. Use instead std::array<std::pair<int, int>, 4>

Also remove statement

std::vector<std::pair<int, int>> ship_loc;      //Ship location

from the constructor. It is a local variable. You need to fill data member ship_loc instead of this local variable.

Vlad from Moscow
  • 301,070
  • 26
  • 186
  • 335
  • I did the 'friend' function, combined with the last comment by @AndyG. I compiles fine (woohoo!) but when I run it I get a segmentation fault. I've read that this is usually caused by trying to access a piece of memory that I'm not allowed to access, or at least not allowed to do with it what I'm trying to do. I wish it gave me some clue as to where the error comes from. Any ideas? – UnworthyToast Mar 22 '14 at 00:37
  • @UnworthyToast See my uopdated post. – Vlad from Moscow Mar 22 '14 at 10:48
0

You're overloading operator<< incorrectly, the signature you want is something like:

std::ostream& operator<<(std::ostream& os, const Ship& s);
Paul Evans
  • 27,315
  • 3
  • 37
  • 54