Franchesca homework (1 and 2) #1
Conversation
homework01/fran/vector.cc
Outdated
| Vector::Vector() | ||
| { | ||
| mX = 0; | ||
| mY = 0; |
There was a problem hiding this comment.
As discussed Thursday, it's good practice to use initializer list, even though in this case it would result into the exact same machine code.
homework01/fran/vector.cc
Outdated
| Vector::Vector(int x, int y) | ||
| { | ||
| mX = x; | ||
| mY = y; |
There was a problem hiding this comment.
I think it was a good choice to not use default values for parameters - what is even better is to call this constructor from the previous one, see delegating constructors: https://en.cppreference.com/w/cpp/language/initializer_list#Delegating_constructor
homework01/fran/vector.cc
Outdated
| mY = y; | ||
| } | ||
|
|
||
| int Vector::GetX() |
There was a problem hiding this comment.
This method does not modify the object state, it should be const
homework01/fran/vector.cc
Outdated
| return mX; | ||
| } | ||
|
|
||
| int Vector::GetY() |
homework01/fran/vector.cc
Outdated
| return mY; | ||
| } | ||
|
|
||
| void Vector::Add(Vector& vector) |
There was a problem hiding this comment.
vector is not modified: it should be const Vector&
homework01/fran/vector.h
Outdated
| class Vector | ||
| { | ||
| public: | ||
| Vector(); |
There was a problem hiding this comment.
The indentation mixes tabs and spaces
| mY(y) | ||
| {} | ||
|
|
||
| const int Vector::GetX() |
There was a problem hiding this comment.
The method should be const, not the returned value. We will discuss about this Thursday.
| @@ -0,0 +1,24 @@ | |||
| #include "vector.h" | |||
|
|
|||
| Vector::Vector(): Vector(0, 0){} | |||
There was a problem hiding this comment.
Optiver's style would be to write
Vector::Vector() :
Vector(0, 0)
{}| { | ||
| mX += rhs.mX; | ||
| mY += rhs.mY; | ||
| return *this; |
There was a problem hiding this comment.
Rule of thumb: only one liners in header files,the rest goes to the cc!
| @@ -0,0 +1,39 @@ | |||
| #include <iostream> | |||
| Vector operator+(const Vector& rhs) const | ||
| { | ||
| return Vector(mX + rhs.mX, mY + rhs.mY); | ||
| } |
There was a problem hiding this comment.
This perfectly works. Did you consider this alternative (free function)?
Vector operator+(Vector lhs, const Vector& rhs)
{
lhs += rhs;
return lhs;
}| return *this; | ||
| } | ||
|
|
||
| friend std::ostream& operator<<(std::ostream& stream, const Vector& rhs) |
There was a problem hiding this comment.
Does it need to be friend? Wouldn't you be able to write this code without?
No description provided.