operators in C++ class

Question

I have the following code and it keeps giving me errors and I'm not really sure what to do because i have the operators declared.

#ifndef BOOK_H
#define BOOK_H
#include<vector>
#include<string>
using namespace std;
class Book {
public:
    Book();
    Book(vector<string>*, string, int);
    Book(const Book&);
    ~Book();
    Book& operator=(const Book&);
    void update(vector<string>*);
    void update(string); 
    void update(int); 
    int getYear() const{
        return year;
    };
    string getTitle() const{
        return title;
    };
    bool operator==(const Book&);
    bool operator!=(const Book&);
    friend ostream& operator<<(ostream&, const Book&);
    void getAuthors();
private:
    vector<string>* authors;
    string title;
    int year;
};

#endif


#include "Book.h"
#include <vector>
#include <string>
#include <cstdlib>
using namespace std;
Book::Book():year(0), title(NULL), authors(NULL){}
Book::Book(vector<string>* bookauthors,string booktitle, int bookyear ){
    authors = bookauthors;
    title = booktitle;
    year = bookyear;
}
Book::Book(const Book& aBook){
    authors = aBook.authors;
    title = aBook.title;
    year = aBook.year;
}
bool Book::operator==(const Book &aBook){
    if(getYear() == aBook.getYear() && getTitle() == aBook.getTitle())
        return true;
    else return false;
}
bool Book::operator != (const Book &aBook){
    if(getYear() != aBook.getYear() && getTitle() == aBook.getTitle())
        return true;
    else return false;
}
Book& Book::operator =(const Book& rhs){
    if(this != rhs){
        authors = rhs.authors;
        title = rhs.title;
        year = rhs.year;
    }
    return *this;
}
void Book::update(int newyear){
    year = newyear;
}
void Book::update(string newtitle){
    title = newtitle;    
}
void Book::update(vector<string>* newauthors){
    authors = newauthors;
}
ostream& operator<<(ostream& os, const Book& b){
    os<<b.getTitle()<<"/"<<b.getYear();
    return os;
}

and i keep getting this error and i'm not sure how to fix it.

Book.cc: In member function 'Book& Book::operator=(const Book&)':
Book.cc:28: error: no match for 'operator!=' in 'this != rhs'
Book.cc: In function 'std::ostream& operator<<(std::ostream&, const Book&)':
Book.cc:45: error: no match for 'operator<<' in 'std::operator<< [with _CharT = char,   _Traits = std::char_traits<char>, _Alloc = std::allocator<char>](((std::basic_ostream<char,   std::char_traits<char> >&)((std::basic_ostream<char, std::char_traits<char> >*)os)), ((const  std::basic_string<char, std::char_traits<char>, std::allocator<char> >&)((const std::basic_string<char, std::char_traits<char>, std::allocator<char> >*)(& Book::getTitle()  const())))) << "/"'
Book.cc:44: note: candidates are: std::ostream& operator<<(std::ostream&, const Book&)

Thanks for the help.

@JohnDibling maybe it is because of that "The C++ programming language" book, or that "Effective C++ one". It might have become unfashionable, but it is still advice given by reputable sources. — juanchopanza, Oct 11 '12 at 20:27

marton78 · Answer 1 · 2012-10-11T18:43:51.553

3

Replace

bool operator==(const Book&);
bool operator!=(const Book&);
friend ostream& operator<<(ostream&, const Book&);

by

bool operator==(const Book&) const;
bool operator!=(const Book&) const;
friend std::ostream& operator<<(std::ostream&, const Book&);

And when checking for equality, try

if (this != &rhs)

or

if (*this != rhs)

depending on whether you want to do a deep or a shallow compare.

edited Oct 11 '12 at 18:43

answered Oct 11 '12 at 18:31

marton78

3,899
2
27
38

He wants a shallow comparison there, it's a common (bad) idiom. – Mooing Duck Oct 11 '12 at 18:35
IMHO doing the comparison at all is a bad idea, see [this popular answer here](http://stackoverflow.com/a/9322542/728847) – marton78 Oct 11 '12 at 18:37
So it is fine to self-assign? – juanchopanza Oct 11 '12 at 18:39
It's a matter of taste. Well designed code should not break when self assigning, so it's just a question of performance. Usually, self-assignment is the exception rather than the rule, so to optimize the frequent case you could skip the check. – marton78 Oct 11 '12 at 18:41
@marton78: It's not a matter of taste -- checking for self assignment is bad code. Any code that does something like `x = x;` is obviously broken. Masking that is just ignoring whatever problems exist and carrying on as if nothing had happened. For much the same reason why a global `catch (...)` is bad, so is this. Don't pretend the problem doesn't exist. Fix the code. – John Dibling Oct 11 '12 at 19:08
If nothing else I think the questioner's code is confused on the issue. Whether or not you accept that optimizing self-assignment is wrong, it seems strange to optimize self-assignment but not self-comparison. – Steve Jessop Oct 11 '12 at 21:28
@JohnDibling: I was about to write something similar but thought it'd be bold to say that self assignment is generally never encountered in sane code. I can't think of any situation where self-assignment might occur, but does it really never happen? Maybe in some sorting algorithm, but so rarely that it's not worth checking for? – marton78 Oct 11 '12 at 22:22
@martin78: I think you can argue that if the caller cares about the performance of self-assignment so much that it's worth checking, he can check for himself. The typical caller will care about the performance of non-self assignment (not that the check will make any difference to that unless the copy is trivial already), and the typical maintenance programmer wants to see as few lines of code (and as few special cases) as possible. – Steve Jessop Oct 11 '12 at 22:26

juanchopanza · Answer 2 · 2012-10-11T18:41:01.327

1

For the first error, you are comparing this, which is a Book pointer, to a Book reference. You need to compare this to the address of the argument, i.e. compare pointers:

if(this != &rhs){ .... }

Concerning the second error, I see nothing wrong in the code you posted. However, you don't need to declare the ostream& << operator as friend, since it only accesses public methods.

You should also make your comparison operators == and != const, although this is unrelated to the errors you report.

edited Oct 11 '12 at 18:41

answered Oct 11 '12 at 18:21

juanchopanza

223,364
34
402
480

That will just compare pointers. It won't invoke the class' operator!=. – Graham Perks Oct 11 '12 at 18:26
2

@GrahamPerks right, and that is exactly what is needed to avoid self-assignment. – juanchopanza Oct 11 '12 at 18:26

operators in C++ class

2 Answers2