US3.2 Overloaded ostream operator for Board added #28
Conversation
gitmajo
commented
Feb 24, 2019
gitmajo
commented
- ostream overloaded operator added (please check if it is placed in proper place, or if it should be friend)
- few methods were renamed to better present their purpose
Codecov Report
@@ Coverage Diff @@
## master #28 +/- ##
=====================================
Coverage 100% 100%
=====================================
Files 6 6
Lines 134 136 +2
=====================================
+ Hits 134 136 +2
Continue to review full report at Codecov.
|
|
Branch US3.2 was updated to current state on US2.2. |
| std::string compareBoard = " +--+\n | |\n+--+--+\n| |??|\n+--+--+\n"; | ||
|
|
||
| testing::internal::CaptureStdout(); | ||
| std::cout << board; |
There was a problem hiding this comment.
I'm unable to reproduce this feature in main.cpp by simple writing down code as below:
#include "Board.hpp"
...
Board board(1, 1, {{1}}, {{1}});
std::cout << board;
I got error:
error: no match for ‘operator<<’ (operand types are ‘std::ostream {aka std::basic_ostream<char>}’ and ‘Board’)
Could you look into it?
There was a problem hiding this comment.
Ok, I moved declaration to board.hpp, but I still prefer to keep definition in DisplayBoard.cpp
There was a problem hiding this comment.
What use is to keep it in Displayboard when you can't use it?
There was a problem hiding this comment.
My intention was to keep definition and declaration in displayBoard.hpp / .cpp because it is completely connected to display functionality. I know that you prefer to include only in main board.hpp, so I moved definition of ostream operator to board.hpp.
From on site it would be clear to have declaration and definition in corresponding files (name.cpp & name.hpp).
On the other hand why not to "hide" definition it in DisplayBoard.cpp and do not "litter" Board.cpp.
@LordLukin what is your opinion?
|
Why this PR wants to merge this branch into US2.2 instead of master? |
I set US2.2 as a base to present only changes between US2.2 and US3.2, but I didn't know that it will mean merging from US3.2 to US2.2. You are right, changed. |
jzych
left a comment
jzych
left a comment
There was a problem hiding this comment.
Looks good, only thing is that overload definition shouldn't also be in Board.cpp?
ziobron
left a comment
ziobron
left a comment
There was a problem hiding this comment.
Is this operator<< really needed? Will it be used anywhere? Right now it isn't.
You can always call cout << display(b); when you need to have it on screen.
| void display() const; | ||
| }; | ||
|
|
||
| std::ostream& operator<<(std::ostream& os, const Board& b); |
There was a problem hiding this comment.
Why is it outside of the Board class? I think that task clearly describes that: Overloaded ostream operator for Board - which in my opinion mean that this should be in the class definition.
