50

I want to make a typedef struct called pos (from position) that stores coordinates x and y. I am trying to overload some operators for this struct, but it does not compile.

typedef struct {
    int x;
    int y;

    inline pos operator=(pos a) {
        x=a.x;
        y=a.y;
        return a;
    }

    inline pos operator+(pos a) {
        return {a.x+x,a.y+y};
    }

    inline bool operator==(pos a) {
       if (a.x==x && a.y== y)
          return true;
       else
          return false;
    }
} pos;

I also wanted to know the difference between this:

inline bool operator==(pos a) {
    if(a.x==x && a.y== y)
       return true;
      else
       return false;
}

And this:

bool operator==(pos a) const {
      if(a.x==x && a.y== y)
         return true;
      else
         return false;
}
jogojapan
  • 68,383
  • 11
  • 101
  • 131
tuket
  • 3,232
  • 1
  • 26
  • 41
  • You might find this stack overflow article of interest. http://stackoverflow.com/questions/612328/difference-between-struct-and-typedef-struct-in-c – Richard Chambers Dec 26 '12 at 22:48
  • You will likely want you `pos a` parameter regardless of operator to be a `const pos& a` (i.e. a const reference) rather than a `pos` copy. after you fix your declaration, of course. – WhozCraig Dec 26 '12 at 22:51
  • 1
    Once you solve your problems. You should also post on [codereview](http://codereview.stackexchange.com/) to get comments on the rest of your code. There are some improvements you may want. – Martin York Dec 26 '12 at 22:53

4 Answers4

105

The breakdown of your declaration and its members is somewhat littered:

Remove the typedef

The typedef is neither required, not desired for class/struct declarations in C++. Your members have no knowledge of the declaration of pos as-written, which is core to your current compilation failure.

Change this:

typedef struct {....} pos;

To this:

struct pos { ... };

Remove extraneous inlines

You're both declaring and defining your member operators within the class definition itself. The inline keyword is not needed so long as your implementations remain in their current location (the class definition)


Return references to *this where appropriate

This is related to an abundance of copy-constructions within your implementation that should not be done without a strong reason for doing so. It is related to the expression ideology of the following:

a = b = c;

This assigns c to b, and the resulting value b is then assigned to a. This is not equivalent to the following code, contrary to what you may think:

a = c;
b = c;

Therefore, your assignment operator should be implemented as such:

pos& operator =(const pos& a)
{
    x = a.x;
    y = a.y;
    return *this;
}

Even here, this is not needed. The default copy-assignment operator will do the above for you free of charge (and code! woot!)

Note: there are times where the above should be avoided in favor of the copy/swap idiom. Though not needed for this specific case, it may look like this:

pos& operator=(pos a) // by-value param invokes class copy-ctor
{
    this->swap(a);
    return *this;
}

Then a swap method is implemented:

void pos::swap(pos& obj)
{
    // TODO: swap object guts with obj
}

You do this to utilize the class copy-ctor to make a copy, then utilize exception-safe swapping to perform the exchange. The result is the incoming copy departs (and destroys) your object's old guts, while your object assumes ownership of there's. Read more the copy/swap idiom here, along with the pros and cons therein.


Pass objects by const reference when appropriate

All of your input parameters to all of your members are currently making copies of whatever is being passed at invoke. While it may be trivial for code like this, it can be very expensive for larger object types. An exampleis given here:

Change this:

bool operator==(pos a) const{
    if(a.x==x && a.y== y)return true;
    else return false;
}

To this: (also simplified)

bool operator==(const pos& a) const
{
    return (x == a.x && y == a.y);
}

No copies of anything are made, resulting in more efficient code.


Finally, in answering your question, what is the difference between a member function or operator declared as const and one that is not?

A const member declares that invoking that member will not modifying the underlying object (mutable declarations not withstanding). Only const member functions can be invoked against const objects, or const references and pointers. For example, your operator +() does not modify your local object and thus should be declared as const. Your operator =() clearly modifies the local object, and therefore the operator should not be const.


Summary

struct pos
{
    int x;
    int y;

    // default + parameterized constructor
    pos(int x=0, int y=0) 
        : x(x), y(y)
    {
    }

    // assignment operator modifies object, therefore non-const
    pos& operator=(const pos& a)
    {
        x=a.x;
        y=a.y;
        return *this;
    }

    // addop. doesn't modify object. therefore const.
    pos operator+(const pos& a) const
    {
        return pos(a.x+x, a.y+y);
    }

    // equality comparison. doesn't modify object. therefore const.
    bool operator==(const pos& a) const
    {
        return (x == a.x && y == a.y);
    }
};

EDIT OP wanted to see how an assignment operator chain works. The following demonstrates how this:

a = b = c;

Is equivalent to this:

b = c;
a = b;

And that this does not always equate to this:

a = c;
b = c;

Sample code:

#include <iostream>
#include <string>
using namespace std;

struct obj
{
    std::string name;
    int value;

    obj(const std::string& name, int value)
        : name(name), value(value)
    {
    }

    obj& operator =(const obj& o)
    {
        cout << name << " = " << o.name << endl;
        value = (o.value+1); // note: our value is one more than the rhs.
        return *this;
    }    
};

int main(int argc, char *argv[])
{

    obj a("a", 1), b("b", 2), c("c", 3);

    a = b = c;
    cout << "a.value = " << a.value << endl;
    cout << "b.value = " << b.value << endl;
    cout << "c.value = " << c.value << endl;

    a = c;
    b = c;
    cout << "a.value = " << a.value << endl;
    cout << "b.value = " << b.value << endl;
    cout << "c.value = " << c.value << endl;

    return 0;
}

Output

b = c
a = b
a.value = 5
b.value = 4
c.value = 3
a = c
b = c
a.value = 4
b.value = 4
c.value = 3
Community
  • 1
  • 1
WhozCraig
  • 65,258
  • 11
  • 75
  • 141
  • Ok, thanks. The only thing I don't understand is the part you say "This is not equivalent to the following code, contrary to what you may think", I keep thinking it is equivalent. – tuket Dec 26 '12 at 23:52
  • @user1754322 It is one time that thinking of chained-assignments with parens helps understand what is going on (at least it helps me). `a = (b = c);` The `b = c` performs the assignment; the resulting value of that reduces to just `b` (after assignment), which is then assigned to `a`. I know, it can be confusing sometimes. You're not alone on that regard. – WhozCraig Dec 26 '12 at 23:57
  • I think I understand it, but you said that a=b=c is **not** equivalent to: a = c; b = c; IMO it is. – tuket Dec 27 '12 at 00:02
  • @user1754322 It is equivalent to `b = c; a = b;` I hope that makes sense. For trivial intrinsic types (`int`, etc...) it may seem meaningless, but for complex types with custom assignment operators (yours) it is significant. If you desire I can update this answer with a very trivial example of how that assignment chain works. – WhozCraig Dec 27 '12 at 00:04
  • I don't realy understand how "**b = c; a = b;**" and "**a = c; b = c;**" can be different but if you say so I will have to believe it :/ – tuket Dec 27 '12 at 00:11
  • 2
    @user1754322 see (and try) the sample I posted at the end of this answer. – WhozCraig Dec 27 '12 at 00:17
  • Now with the example that adds 1 it makes sense. I was thinking of = as the common assignment operator, I was forgetting that you can give any sense to an operator. Anyway I think that if you give to the "=" operator the common assignment meaning, then "b = c; a = b;" and "a = c; b = c;" are the same(at least in that case). Thank you very much for the effort! – tuket Dec 27 '12 at 00:37
  • @user1754322 Excellent. It was the order of assignments I wanted to get across more than anything. The valuation is kind of an afterthought. Glad it make sense now. Good luck, sir. – WhozCraig Dec 27 '12 at 00:42
6

Instead of typedef struct { ... } pos; you should be doing struct pos { ... };. The issue here is that you are using the pos type name before it is defined. By moving the name to the top of the struct definition, you are able to use that name within the struct definition itself.

Further, the typedef struct { ... } name; pattern is a C-ism, and doesn't have much place in C++.

To answer your question about inline, there is no difference in this case. When a method is defined within the struct/class definition, it is implicitly declared inline. When you explicitly specify inline, the compiler effectively ignores it because the method is already declared inline.

(inline methods will not trigger a linker error if the same method is defined in multiple object files; the linker will simply ignore all but one of them, assuming that they are all the same implementation. This is the only guaranteed change in behavior with inline methods. Nowadays, they do not affect the compiler's decision regarding whether or not to inline functions; they simply facilitate making the function implementation available in all translation units, which gives the compiler the option to inline the function, if it decides it would be beneficial to do so.)

cdhowie
  • 158,093
  • 24
  • 286
  • 300
4

try this:

struct Pos{
    int x;
    int y;

    inline Pos& operator=(const Pos& other){
        x=other.x;
        y=other.y;
        return *this;
    }

    inline Pos operator+(const Pos& other) const {
        Pos res {x+other.x,y+other.y};
        return res;
    }

    const inline bool operator==(const Pos& other) const {
        return (x==other.x and y == other.y);
    }
 };  
Krozark
  • 862
  • 9
  • 27
  • And what does the `typedef` do here? Just drop it. (The only reason to use `typedef` of a `struct` is C compatibility, and that means no member functions, no friends, etc.) – James Kanze Dec 26 '12 at 23:10
  • I don't understand this "const Pos& other" – tuket Dec 26 '12 at 23:36
  • it to not make a copy, and to be sur that you do not modify "other" inside your methode. By this way g++ (or other) can make same optimisation. – Krozark Dec 26 '12 at 23:39
  • 1
    in fact (Pos other) <= make a copy, (Pos& other) no copy, but you can modify other (like with pointer), (const Pos& other) no copy, and you can't modify other. – Krozark Dec 26 '12 at 23:41
  • This would be a great answer if there was some explanation on _why_ you made the changes you made. – jogojapan Dec 27 '12 at 00:55
  • I don't understand what's the point of the const in `const inline bool`? – forumulator Oct 04 '17 at 12:23
3
  1. bool operator==(pos a) const{ - this method doesn't change object's elements.
  2. bool operator==(pos a) { - it may change object's elements.
ulidtko
  • 14,740
  • 10
  • 56
  • 88
maskotky
  • 54
  • 5