Skip to content
This repository has been archived by the owner on Nov 26, 2024. It is now read-only.

Issue 39 #51

Merged
merged 12 commits into from
Dec 6, 2023
Merged

Issue 39 #51

merged 12 commits into from
Dec 6, 2023

Conversation

martprenger
Copy link
Contributor

we finished issue #39

Copy link
Contributor

@TheoPannetier TheoPannetier left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good work! I see you found a simpler solution to set_position() than should have been, but I'll let it slide - I should have made an issue like #52 as a Depend on this one.
Please do have a look at and commit my suggestions, then you can merge 👍

player.cpp Outdated Show resolved Hide resolved
player.cpp Outdated
m_line_x_position = line_x_position;
}

void player::score(double line_x_position) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
void player::score(double line_x_position) {
void player::score(const double& line_x_position) {

We don't need to change the line position here, so let's be clear about that :)

Comment on lines +22 to +24
void player::set_position(double line_x_position) {
m_line_x_position = line_x_position;
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not the intended solution by the test (the field's line should probably not be a property of the player!), but it does address the test. I'll allow it for now, eventually we'll have to break this (in #52).

player.h Outdated
double get_x() const noexcept { return m_x; }
double get_y() const noexcept { return m_y; }

int get_score() const noexcept;
void set_score(const int new_score);

void set_position(double line_x_position);
void score(double line_x_position);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
void score(double line_x_position);
void score(const double& line_x_position);

player.cpp Outdated
Comment on lines 6 to 7
m_x{0},
m_y{0},
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
m_x{0},
m_y{0},
m_x{0.0},
m_y{0.0},

Good practice: the .0, although useless, confirms to humans this is a float and not an integer 😉

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

now i changed it back there are no problems but when we were working on the issue we had a weird error and this fixed it.

Co-authored-by: Théo Pannetier <[email protected]>
Copy link

codecov bot commented Dec 6, 2023

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (2644446) 100.00% compared to head (1d07f5c) 100.00%.

❗ Current head 1d07f5c differs from pull request most recent head c0f81a1. Consider uploading reports for the commit c0f81a1 to get more accurate results

Additional details and impacted files
@@            Coverage Diff            @@
##           develop       #51   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files            6         6           
  Lines           83       114   +31     
  Branches         0         1    +1     
=========================================
+ Hits            83       114   +31     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@martprenger martprenger merged commit b658482 into develop Dec 6, 2023
2 of 4 checks passed
@martprenger martprenger deleted the issue_39 branch December 6, 2023 17:27
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants