A game called Bocce played on a Cartesian grid
.everyoneloves__top-leaderboard:empty,.everyoneloves__mid-leaderboard:empty,.everyoneloves__bot-mid-leaderboard:empty margin-bottom:0;
$begingroup$
You may know Bocce, so you should know that I've tweaked the rules a little. Three types of balls are thrown: a red type, a blue type, and single black ball called the jack.
These are all on the Cartesian coordinate system. The coordinates of the jack are given, and also distances of an equal amount of red and blue balls from the origins of the system (0, 0). If a ball's distance is larger than the distance of the jack, and if the distance of this ball is larger than the distance of its peer, it's a score. The team with the highest score wins.
Here's the code:
#include <iostream>
#include <map>
#include <vector>
#include <string>
#include <cmath>
#include <array>
#include <algorithm>
typedef std::map<std::string, std::vector<int>> bocce_balls;
typedef std::vector<int> int_vec;
typedef std::array<int, 2> int_arr;
std::string bocce(bocce_balls balls, int_arr jack)
auto distance = std::sqrt(std::pow(jack[0], 2)+std::pow(jack[1], 2));
int red_count = 0;
int blue_count = 0;
int_vec red = balls["red"];
int_vec blue = balls["blue"];
if (blue.size() != red.size())
std::cerr << "Balls were different numbers!" << std::endl;
return "Fail!";
std::sort(blue.begin(), blue.end());
std::sort(red.begin(), red.end());
for (int i = 0; i < blue.size(); ++i)
if (red[i] > distance && red[i] > blue[i])
++red_count;
else if (blue[i] > distance && blue[i] > red[i])
++blue_count;
if (red_count > blue_count)
return "Red Wins!";
else if (blue_count > red_count)
return "Blue Wins!";
else
return "Draw!";
int main()
bocce_balls insert;
insert["blue"] = 2, 9, 3, 4, 5, 1, 10, 2;
insert["red"] = 20, 11, 3, 4, 3, 1, 2, 3;
int_arr dist = 2, 2;
std::cout << bocce(insert, dist) << std::endl;
return 0;
c++ c++14
$endgroup$
add a comment |
$begingroup$
You may know Bocce, so you should know that I've tweaked the rules a little. Three types of balls are thrown: a red type, a blue type, and single black ball called the jack.
These are all on the Cartesian coordinate system. The coordinates of the jack are given, and also distances of an equal amount of red and blue balls from the origins of the system (0, 0). If a ball's distance is larger than the distance of the jack, and if the distance of this ball is larger than the distance of its peer, it's a score. The team with the highest score wins.
Here's the code:
#include <iostream>
#include <map>
#include <vector>
#include <string>
#include <cmath>
#include <array>
#include <algorithm>
typedef std::map<std::string, std::vector<int>> bocce_balls;
typedef std::vector<int> int_vec;
typedef std::array<int, 2> int_arr;
std::string bocce(bocce_balls balls, int_arr jack)
auto distance = std::sqrt(std::pow(jack[0], 2)+std::pow(jack[1], 2));
int red_count = 0;
int blue_count = 0;
int_vec red = balls["red"];
int_vec blue = balls["blue"];
if (blue.size() != red.size())
std::cerr << "Balls were different numbers!" << std::endl;
return "Fail!";
std::sort(blue.begin(), blue.end());
std::sort(red.begin(), red.end());
for (int i = 0; i < blue.size(); ++i)
if (red[i] > distance && red[i] > blue[i])
++red_count;
else if (blue[i] > distance && blue[i] > red[i])
++blue_count;
if (red_count > blue_count)
return "Red Wins!";
else if (blue_count > red_count)
return "Blue Wins!";
else
return "Draw!";
int main()
bocce_balls insert;
insert["blue"] = 2, 9, 3, 4, 5, 1, 10, 2;
insert["red"] = 20, 11, 3, 4, 3, 1, 2, 3;
int_arr dist = 2, 2;
std::cout << bocce(insert, dist) << std::endl;
return 0;
c++ c++14
$endgroup$
1
$begingroup$
Don't use std::pow for squaring, you can hardly be less efficient... Just multiply the value with itself instead.
$endgroup$
– Aconcagua
Nov 16 '18 at 9:08
add a comment |
$begingroup$
You may know Bocce, so you should know that I've tweaked the rules a little. Three types of balls are thrown: a red type, a blue type, and single black ball called the jack.
These are all on the Cartesian coordinate system. The coordinates of the jack are given, and also distances of an equal amount of red and blue balls from the origins of the system (0, 0). If a ball's distance is larger than the distance of the jack, and if the distance of this ball is larger than the distance of its peer, it's a score. The team with the highest score wins.
Here's the code:
#include <iostream>
#include <map>
#include <vector>
#include <string>
#include <cmath>
#include <array>
#include <algorithm>
typedef std::map<std::string, std::vector<int>> bocce_balls;
typedef std::vector<int> int_vec;
typedef std::array<int, 2> int_arr;
std::string bocce(bocce_balls balls, int_arr jack)
auto distance = std::sqrt(std::pow(jack[0], 2)+std::pow(jack[1], 2));
int red_count = 0;
int blue_count = 0;
int_vec red = balls["red"];
int_vec blue = balls["blue"];
if (blue.size() != red.size())
std::cerr << "Balls were different numbers!" << std::endl;
return "Fail!";
std::sort(blue.begin(), blue.end());
std::sort(red.begin(), red.end());
for (int i = 0; i < blue.size(); ++i)
if (red[i] > distance && red[i] > blue[i])
++red_count;
else if (blue[i] > distance && blue[i] > red[i])
++blue_count;
if (red_count > blue_count)
return "Red Wins!";
else if (blue_count > red_count)
return "Blue Wins!";
else
return "Draw!";
int main()
bocce_balls insert;
insert["blue"] = 2, 9, 3, 4, 5, 1, 10, 2;
insert["red"] = 20, 11, 3, 4, 3, 1, 2, 3;
int_arr dist = 2, 2;
std::cout << bocce(insert, dist) << std::endl;
return 0;
c++ c++14
$endgroup$
You may know Bocce, so you should know that I've tweaked the rules a little. Three types of balls are thrown: a red type, a blue type, and single black ball called the jack.
These are all on the Cartesian coordinate system. The coordinates of the jack are given, and also distances of an equal amount of red and blue balls from the origins of the system (0, 0). If a ball's distance is larger than the distance of the jack, and if the distance of this ball is larger than the distance of its peer, it's a score. The team with the highest score wins.
Here's the code:
#include <iostream>
#include <map>
#include <vector>
#include <string>
#include <cmath>
#include <array>
#include <algorithm>
typedef std::map<std::string, std::vector<int>> bocce_balls;
typedef std::vector<int> int_vec;
typedef std::array<int, 2> int_arr;
std::string bocce(bocce_balls balls, int_arr jack)
auto distance = std::sqrt(std::pow(jack[0], 2)+std::pow(jack[1], 2));
int red_count = 0;
int blue_count = 0;
int_vec red = balls["red"];
int_vec blue = balls["blue"];
if (blue.size() != red.size())
std::cerr << "Balls were different numbers!" << std::endl;
return "Fail!";
std::sort(blue.begin(), blue.end());
std::sort(red.begin(), red.end());
for (int i = 0; i < blue.size(); ++i)
if (red[i] > distance && red[i] > blue[i])
++red_count;
else if (blue[i] > distance && blue[i] > red[i])
++blue_count;
if (red_count > blue_count)
return "Red Wins!";
else if (blue_count > red_count)
return "Blue Wins!";
else
return "Draw!";
int main()
bocce_balls insert;
insert["blue"] = 2, 9, 3, 4, 5, 1, 10, 2;
insert["red"] = 20, 11, 3, 4, 3, 1, 2, 3;
int_arr dist = 2, 2;
std::cout << bocce(insert, dist) << std::endl;
return 0;
c++ c++14
c++ c++14
edited Nov 16 '18 at 8:49
Josay
26k14087
26k14087
asked Nov 16 '18 at 7:29
ChubakBidpaaChubakBidpaa
12217
12217
1
$begingroup$
Don't use std::pow for squaring, you can hardly be less efficient... Just multiply the value with itself instead.
$endgroup$
– Aconcagua
Nov 16 '18 at 9:08
add a comment |
1
$begingroup$
Don't use std::pow for squaring, you can hardly be less efficient... Just multiply the value with itself instead.
$endgroup$
– Aconcagua
Nov 16 '18 at 9:08
1
1
$begingroup$
Don't use std::pow for squaring, you can hardly be less efficient... Just multiply the value with itself instead.
$endgroup$
– Aconcagua
Nov 16 '18 at 9:08
$begingroup$
Don't use std::pow for squaring, you can hardly be less efficient... Just multiply the value with itself instead.
$endgroup$
– Aconcagua
Nov 16 '18 at 9:08
add a comment |
2 Answers
2
active
oldest
votes
$begingroup$
Bocce, as far as I know it, counts all the balls of one player that are closer to the jack than the closest one of his opponent.
That would mean that the distances from origin are irrelevant, instead, you need the distances from the balls to the jack:
auto dx = ball.x - jack.x;
auto dy = ball.y - jack.y;
auto distance2 = dx*dx + dy*dy;
Note that I avoided std::pow
in favour to multiplying with itself, as std::pow
inefficient for this purpose.
Note, too, that as the root function is strictly monotonely rising, you don't need to calculate it, but you can compare the squares as well and still will get the same results.
I personally strongly recommend that you create a class Ball
containing three members: x, y for the coordinates and distance for the distance to jack:
class Ball
int x, y;
int distanceToJack2;
;
I'm not too happy with int as data type - it depends on the range of valid coordinates, though, if it is suitable or not. Assuming we have 32-bit int, then if the valid ranges for x and y can be covered with 16 bit, you will be safe from overflow while multiplying (this would yield undefined behaviour!), otherwise you'd need to calculate in next larger data type. For selecting the correct data types more safely, you might prefer the data types from <cstdint>
header, e. g.:
int32_t x, y;
int64_t distanceToJack2;
Or you simply have coordinates and distance in double right away. Be aware that with double, you can quickly get issues due to rounding, so if you compare for one value being less than the other, you should consider them only so if difference is larger than some yet to be defined epsilon value (minimum distance for two values for not considering them equal). You could get some hints to from this question. Sure, it is java, but as long as both C++ and Java follow IEEE 754 for floating point values, it applies for both languages equally. On the other hand, you could then use std::hypot
for calculating the distances in a very safe manner (thanks Toby for the hint).
Then you might add:
void Ball::calculateDistance(Ball const& jack)
uint64_t dx = static_cast<uint64_t>(x) - jack.x;
uint64_t dy = static_cast<uint64_t>(y) - jack.y;
distanceToJack2 = dx*dx + dy*dy;
Finally, you can sort your balls according to the distances to jack and select all balls from one vector of which the distance is smaller than the distance of the first element in the respective other vector.
One last point: std::string is a rather bad key type for the map if you only have two fix values (beware of typos, additionally, map mangement is rather expensive). You might prefer an enum instead: enum class Color RED, BLUE ;
and use this one as key instead. Even better: don't use a map at all, instead:
std::string bocce(std::vector<Ball>& red, std::vector<Ball>& blue, Ball const& jack);
Note that as you need to calculate the distances, which are stored in the balls, you cannot have const references for the two vectors (which otherwise would have been preferrable). From point of view of design, this is at least questionable (but at least, it's quite some improvement already). Ideally, the design would allow to pass in const vectors, but that would require some overhead, so for pragmatism, we might decide to live with the current design...
$endgroup$
$begingroup$
All these help me learn C++ better. Thanks.
$endgroup$
– ChubakBidpaa
Nov 16 '18 at 10:27
add a comment |
$begingroup$
typedef std::map<std::string, std::vector<int>> bocce_balls;
Are you interested in the ordering of the container? There really is no need to use std::set
to wrap the ball distances for each player. std::unordered_map
would be faster (consumes more space!), but do you even need a map-type? Have you considered using an array of vectors to represent the distances for each player? Do you need to wrap the distances or can you just pass each player as arguments?
Should the key be a string or an enumeration? Strings are great when you need to make keys on the fly at run-time. At compile-time, spelling errors with strings will result in new entries being created in mutable maps. With strongly typed enumerations, the possible range of keys is predefined.
Should distances be stored as integers or as a floating-point type?
std::string bocce(bocce_balls balls, int_arr jack)
Pick descriptive names that help readers understand what your function is supposed to do. bocce
is the name of the game. What does this function do? Calculates the winner for a game of bocce. Cartesian point makes it clear what the type of jack
should be. Balls is holding the results for one frame in a game of bocce.
auto distance = std::sqrt(std::pow(jack[0], 2)+std::pow(jack[1], 2));
Use const
to define objects with values that do not change after construction. const
is useful for providing an immutable view of mutable data. This allows you to clarify to readers that the variable will not be modified and it prevents surprises when someone unexpectedly changes the objects value.
Also, distance from an origin exists in <cmath>
. See std::hypot
.
int_vec red = balls["red"];
int_vec blue = balls["blue"];
Key-access through a map returns a reference to the mapped value. Here, you've opted into making a copy of the mapped value. If you look back at your function signature, bocce_balls
is passed by value. Another copy. I'd advise that you pass the parameter by reference-to-const
as to prevent accidental modifications to the map. Keep the copies of red
and blue
here as you'll need those copies for the sort.
for (int i = 0; i < blue.size(); ++i)
if (red[i] > distance && red[i] > blue[i])
++red_count;
else if (blue[i] > distance && blue[i] > red[i])
++blue_count;
i
is a signed integer. blue.size()
returns an unsigned integer. Each loops does a signed/unsigned comparison. Every time you index with i
, the subscript operator is expecting an unsigned integer which leads to implicit conversions that change signedness. Turn up your warning levels.
$endgroup$
$begingroup$
Although it is against the rules to thank people, I shall thank you for taking your time to correct my code. So thanks.
$endgroup$
– ChubakBidpaa
Nov 16 '18 at 10:26
add a comment |
Your Answer
StackExchange.ifUsing("editor", function ()
return StackExchange.using("mathjaxEditing", function ()
StackExchange.MarkdownEditor.creationCallbacks.add(function (editor, postfix)
StackExchange.mathjaxEditing.prepareWmdForMathJax(editor, postfix, [["\$", "\$"]]);
);
);
, "mathjax-editing");
StackExchange.ifUsing("editor", function ()
StackExchange.using("externalEditor", function ()
StackExchange.using("snippets", function ()
StackExchange.snippets.init();
);
);
, "code-snippets");
StackExchange.ready(function()
var channelOptions =
tags: "".split(" "),
id: "196"
;
initTagRenderer("".split(" "), "".split(" "), channelOptions);
StackExchange.using("externalEditor", function()
// Have to fire editor after snippets, if snippets enabled
if (StackExchange.settings.snippets.snippetsEnabled)
StackExchange.using("snippets", function()
createEditor();
);
else
createEditor();
);
function createEditor()
StackExchange.prepareEditor(
heartbeatType: 'answer',
autoActivateHeartbeat: false,
convertImagesToLinks: false,
noModals: true,
showLowRepImageUploadWarning: true,
reputationToPostImages: null,
bindNavPrevention: true,
postfix: "",
imageUploader:
brandingHtml: "Powered by u003ca class="icon-imgur-white" href="https://imgur.com/"u003eu003c/au003e",
contentPolicyHtml: "User contributions licensed under u003ca href="https://creativecommons.org/licenses/by-sa/3.0/"u003ecc by-sa 3.0 with attribution requiredu003c/au003e u003ca href="https://stackoverflow.com/legal/content-policy"u003e(content policy)u003c/au003e",
allowUrls: true
,
onDemand: true,
discardSelector: ".discard-answer"
,immediatelyShowMarkdownHelp:true
);
);
Sign up or log in
StackExchange.ready(function ()
StackExchange.helpers.onClickDraftSave('#login-link');
);
Sign up using Google
Sign up using Facebook
Sign up using Email and Password
Post as a guest
Required, but never shown
StackExchange.ready(
function ()
StackExchange.openid.initPostLogin('.new-post-login', 'https%3a%2f%2fcodereview.stackexchange.com%2fquestions%2f207781%2fa-game-called-bocce-played-on-a-cartesian-grid%23new-answer', 'question_page');
);
Post as a guest
Required, but never shown
2 Answers
2
active
oldest
votes
2 Answers
2
active
oldest
votes
active
oldest
votes
active
oldest
votes
$begingroup$
Bocce, as far as I know it, counts all the balls of one player that are closer to the jack than the closest one of his opponent.
That would mean that the distances from origin are irrelevant, instead, you need the distances from the balls to the jack:
auto dx = ball.x - jack.x;
auto dy = ball.y - jack.y;
auto distance2 = dx*dx + dy*dy;
Note that I avoided std::pow
in favour to multiplying with itself, as std::pow
inefficient for this purpose.
Note, too, that as the root function is strictly monotonely rising, you don't need to calculate it, but you can compare the squares as well and still will get the same results.
I personally strongly recommend that you create a class Ball
containing three members: x, y for the coordinates and distance for the distance to jack:
class Ball
int x, y;
int distanceToJack2;
;
I'm not too happy with int as data type - it depends on the range of valid coordinates, though, if it is suitable or not. Assuming we have 32-bit int, then if the valid ranges for x and y can be covered with 16 bit, you will be safe from overflow while multiplying (this would yield undefined behaviour!), otherwise you'd need to calculate in next larger data type. For selecting the correct data types more safely, you might prefer the data types from <cstdint>
header, e. g.:
int32_t x, y;
int64_t distanceToJack2;
Or you simply have coordinates and distance in double right away. Be aware that with double, you can quickly get issues due to rounding, so if you compare for one value being less than the other, you should consider them only so if difference is larger than some yet to be defined epsilon value (minimum distance for two values for not considering them equal). You could get some hints to from this question. Sure, it is java, but as long as both C++ and Java follow IEEE 754 for floating point values, it applies for both languages equally. On the other hand, you could then use std::hypot
for calculating the distances in a very safe manner (thanks Toby for the hint).
Then you might add:
void Ball::calculateDistance(Ball const& jack)
uint64_t dx = static_cast<uint64_t>(x) - jack.x;
uint64_t dy = static_cast<uint64_t>(y) - jack.y;
distanceToJack2 = dx*dx + dy*dy;
Finally, you can sort your balls according to the distances to jack and select all balls from one vector of which the distance is smaller than the distance of the first element in the respective other vector.
One last point: std::string is a rather bad key type for the map if you only have two fix values (beware of typos, additionally, map mangement is rather expensive). You might prefer an enum instead: enum class Color RED, BLUE ;
and use this one as key instead. Even better: don't use a map at all, instead:
std::string bocce(std::vector<Ball>& red, std::vector<Ball>& blue, Ball const& jack);
Note that as you need to calculate the distances, which are stored in the balls, you cannot have const references for the two vectors (which otherwise would have been preferrable). From point of view of design, this is at least questionable (but at least, it's quite some improvement already). Ideally, the design would allow to pass in const vectors, but that would require some overhead, so for pragmatism, we might decide to live with the current design...
$endgroup$
$begingroup$
All these help me learn C++ better. Thanks.
$endgroup$
– ChubakBidpaa
Nov 16 '18 at 10:27
add a comment |
$begingroup$
Bocce, as far as I know it, counts all the balls of one player that are closer to the jack than the closest one of his opponent.
That would mean that the distances from origin are irrelevant, instead, you need the distances from the balls to the jack:
auto dx = ball.x - jack.x;
auto dy = ball.y - jack.y;
auto distance2 = dx*dx + dy*dy;
Note that I avoided std::pow
in favour to multiplying with itself, as std::pow
inefficient for this purpose.
Note, too, that as the root function is strictly monotonely rising, you don't need to calculate it, but you can compare the squares as well and still will get the same results.
I personally strongly recommend that you create a class Ball
containing three members: x, y for the coordinates and distance for the distance to jack:
class Ball
int x, y;
int distanceToJack2;
;
I'm not too happy with int as data type - it depends on the range of valid coordinates, though, if it is suitable or not. Assuming we have 32-bit int, then if the valid ranges for x and y can be covered with 16 bit, you will be safe from overflow while multiplying (this would yield undefined behaviour!), otherwise you'd need to calculate in next larger data type. For selecting the correct data types more safely, you might prefer the data types from <cstdint>
header, e. g.:
int32_t x, y;
int64_t distanceToJack2;
Or you simply have coordinates and distance in double right away. Be aware that with double, you can quickly get issues due to rounding, so if you compare for one value being less than the other, you should consider them only so if difference is larger than some yet to be defined epsilon value (minimum distance for two values for not considering them equal). You could get some hints to from this question. Sure, it is java, but as long as both C++ and Java follow IEEE 754 for floating point values, it applies for both languages equally. On the other hand, you could then use std::hypot
for calculating the distances in a very safe manner (thanks Toby for the hint).
Then you might add:
void Ball::calculateDistance(Ball const& jack)
uint64_t dx = static_cast<uint64_t>(x) - jack.x;
uint64_t dy = static_cast<uint64_t>(y) - jack.y;
distanceToJack2 = dx*dx + dy*dy;
Finally, you can sort your balls according to the distances to jack and select all balls from one vector of which the distance is smaller than the distance of the first element in the respective other vector.
One last point: std::string is a rather bad key type for the map if you only have two fix values (beware of typos, additionally, map mangement is rather expensive). You might prefer an enum instead: enum class Color RED, BLUE ;
and use this one as key instead. Even better: don't use a map at all, instead:
std::string bocce(std::vector<Ball>& red, std::vector<Ball>& blue, Ball const& jack);
Note that as you need to calculate the distances, which are stored in the balls, you cannot have const references for the two vectors (which otherwise would have been preferrable). From point of view of design, this is at least questionable (but at least, it's quite some improvement already). Ideally, the design would allow to pass in const vectors, but that would require some overhead, so for pragmatism, we might decide to live with the current design...
$endgroup$
$begingroup$
All these help me learn C++ better. Thanks.
$endgroup$
– ChubakBidpaa
Nov 16 '18 at 10:27
add a comment |
$begingroup$
Bocce, as far as I know it, counts all the balls of one player that are closer to the jack than the closest one of his opponent.
That would mean that the distances from origin are irrelevant, instead, you need the distances from the balls to the jack:
auto dx = ball.x - jack.x;
auto dy = ball.y - jack.y;
auto distance2 = dx*dx + dy*dy;
Note that I avoided std::pow
in favour to multiplying with itself, as std::pow
inefficient for this purpose.
Note, too, that as the root function is strictly monotonely rising, you don't need to calculate it, but you can compare the squares as well and still will get the same results.
I personally strongly recommend that you create a class Ball
containing three members: x, y for the coordinates and distance for the distance to jack:
class Ball
int x, y;
int distanceToJack2;
;
I'm not too happy with int as data type - it depends on the range of valid coordinates, though, if it is suitable or not. Assuming we have 32-bit int, then if the valid ranges for x and y can be covered with 16 bit, you will be safe from overflow while multiplying (this would yield undefined behaviour!), otherwise you'd need to calculate in next larger data type. For selecting the correct data types more safely, you might prefer the data types from <cstdint>
header, e. g.:
int32_t x, y;
int64_t distanceToJack2;
Or you simply have coordinates and distance in double right away. Be aware that with double, you can quickly get issues due to rounding, so if you compare for one value being less than the other, you should consider them only so if difference is larger than some yet to be defined epsilon value (minimum distance for two values for not considering them equal). You could get some hints to from this question. Sure, it is java, but as long as both C++ and Java follow IEEE 754 for floating point values, it applies for both languages equally. On the other hand, you could then use std::hypot
for calculating the distances in a very safe manner (thanks Toby for the hint).
Then you might add:
void Ball::calculateDistance(Ball const& jack)
uint64_t dx = static_cast<uint64_t>(x) - jack.x;
uint64_t dy = static_cast<uint64_t>(y) - jack.y;
distanceToJack2 = dx*dx + dy*dy;
Finally, you can sort your balls according to the distances to jack and select all balls from one vector of which the distance is smaller than the distance of the first element in the respective other vector.
One last point: std::string is a rather bad key type for the map if you only have two fix values (beware of typos, additionally, map mangement is rather expensive). You might prefer an enum instead: enum class Color RED, BLUE ;
and use this one as key instead. Even better: don't use a map at all, instead:
std::string bocce(std::vector<Ball>& red, std::vector<Ball>& blue, Ball const& jack);
Note that as you need to calculate the distances, which are stored in the balls, you cannot have const references for the two vectors (which otherwise would have been preferrable). From point of view of design, this is at least questionable (but at least, it's quite some improvement already). Ideally, the design would allow to pass in const vectors, but that would require some overhead, so for pragmatism, we might decide to live with the current design...
$endgroup$
Bocce, as far as I know it, counts all the balls of one player that are closer to the jack than the closest one of his opponent.
That would mean that the distances from origin are irrelevant, instead, you need the distances from the balls to the jack:
auto dx = ball.x - jack.x;
auto dy = ball.y - jack.y;
auto distance2 = dx*dx + dy*dy;
Note that I avoided std::pow
in favour to multiplying with itself, as std::pow
inefficient for this purpose.
Note, too, that as the root function is strictly monotonely rising, you don't need to calculate it, but you can compare the squares as well and still will get the same results.
I personally strongly recommend that you create a class Ball
containing three members: x, y for the coordinates and distance for the distance to jack:
class Ball
int x, y;
int distanceToJack2;
;
I'm not too happy with int as data type - it depends on the range of valid coordinates, though, if it is suitable or not. Assuming we have 32-bit int, then if the valid ranges for x and y can be covered with 16 bit, you will be safe from overflow while multiplying (this would yield undefined behaviour!), otherwise you'd need to calculate in next larger data type. For selecting the correct data types more safely, you might prefer the data types from <cstdint>
header, e. g.:
int32_t x, y;
int64_t distanceToJack2;
Or you simply have coordinates and distance in double right away. Be aware that with double, you can quickly get issues due to rounding, so if you compare for one value being less than the other, you should consider them only so if difference is larger than some yet to be defined epsilon value (minimum distance for two values for not considering them equal). You could get some hints to from this question. Sure, it is java, but as long as both C++ and Java follow IEEE 754 for floating point values, it applies for both languages equally. On the other hand, you could then use std::hypot
for calculating the distances in a very safe manner (thanks Toby for the hint).
Then you might add:
void Ball::calculateDistance(Ball const& jack)
uint64_t dx = static_cast<uint64_t>(x) - jack.x;
uint64_t dy = static_cast<uint64_t>(y) - jack.y;
distanceToJack2 = dx*dx + dy*dy;
Finally, you can sort your balls according to the distances to jack and select all balls from one vector of which the distance is smaller than the distance of the first element in the respective other vector.
One last point: std::string is a rather bad key type for the map if you only have two fix values (beware of typos, additionally, map mangement is rather expensive). You might prefer an enum instead: enum class Color RED, BLUE ;
and use this one as key instead. Even better: don't use a map at all, instead:
std::string bocce(std::vector<Ball>& red, std::vector<Ball>& blue, Ball const& jack);
Note that as you need to calculate the distances, which are stored in the balls, you cannot have const references for the two vectors (which otherwise would have been preferrable). From point of view of design, this is at least questionable (but at least, it's quite some improvement already). Ideally, the design would allow to pass in const vectors, but that would require some overhead, so for pragmatism, we might decide to live with the current design...
edited Nov 16 '18 at 9:55
answered Nov 16 '18 at 9:48
AconcaguaAconcagua
31116
31116
$begingroup$
All these help me learn C++ better. Thanks.
$endgroup$
– ChubakBidpaa
Nov 16 '18 at 10:27
add a comment |
$begingroup$
All these help me learn C++ better. Thanks.
$endgroup$
– ChubakBidpaa
Nov 16 '18 at 10:27
$begingroup$
All these help me learn C++ better. Thanks.
$endgroup$
– ChubakBidpaa
Nov 16 '18 at 10:27
$begingroup$
All these help me learn C++ better. Thanks.
$endgroup$
– ChubakBidpaa
Nov 16 '18 at 10:27
add a comment |
$begingroup$
typedef std::map<std::string, std::vector<int>> bocce_balls;
Are you interested in the ordering of the container? There really is no need to use std::set
to wrap the ball distances for each player. std::unordered_map
would be faster (consumes more space!), but do you even need a map-type? Have you considered using an array of vectors to represent the distances for each player? Do you need to wrap the distances or can you just pass each player as arguments?
Should the key be a string or an enumeration? Strings are great when you need to make keys on the fly at run-time. At compile-time, spelling errors with strings will result in new entries being created in mutable maps. With strongly typed enumerations, the possible range of keys is predefined.
Should distances be stored as integers or as a floating-point type?
std::string bocce(bocce_balls balls, int_arr jack)
Pick descriptive names that help readers understand what your function is supposed to do. bocce
is the name of the game. What does this function do? Calculates the winner for a game of bocce. Cartesian point makes it clear what the type of jack
should be. Balls is holding the results for one frame in a game of bocce.
auto distance = std::sqrt(std::pow(jack[0], 2)+std::pow(jack[1], 2));
Use const
to define objects with values that do not change after construction. const
is useful for providing an immutable view of mutable data. This allows you to clarify to readers that the variable will not be modified and it prevents surprises when someone unexpectedly changes the objects value.
Also, distance from an origin exists in <cmath>
. See std::hypot
.
int_vec red = balls["red"];
int_vec blue = balls["blue"];
Key-access through a map returns a reference to the mapped value. Here, you've opted into making a copy of the mapped value. If you look back at your function signature, bocce_balls
is passed by value. Another copy. I'd advise that you pass the parameter by reference-to-const
as to prevent accidental modifications to the map. Keep the copies of red
and blue
here as you'll need those copies for the sort.
for (int i = 0; i < blue.size(); ++i)
if (red[i] > distance && red[i] > blue[i])
++red_count;
else if (blue[i] > distance && blue[i] > red[i])
++blue_count;
i
is a signed integer. blue.size()
returns an unsigned integer. Each loops does a signed/unsigned comparison. Every time you index with i
, the subscript operator is expecting an unsigned integer which leads to implicit conversions that change signedness. Turn up your warning levels.
$endgroup$
$begingroup$
Although it is against the rules to thank people, I shall thank you for taking your time to correct my code. So thanks.
$endgroup$
– ChubakBidpaa
Nov 16 '18 at 10:26
add a comment |
$begingroup$
typedef std::map<std::string, std::vector<int>> bocce_balls;
Are you interested in the ordering of the container? There really is no need to use std::set
to wrap the ball distances for each player. std::unordered_map
would be faster (consumes more space!), but do you even need a map-type? Have you considered using an array of vectors to represent the distances for each player? Do you need to wrap the distances or can you just pass each player as arguments?
Should the key be a string or an enumeration? Strings are great when you need to make keys on the fly at run-time. At compile-time, spelling errors with strings will result in new entries being created in mutable maps. With strongly typed enumerations, the possible range of keys is predefined.
Should distances be stored as integers or as a floating-point type?
std::string bocce(bocce_balls balls, int_arr jack)
Pick descriptive names that help readers understand what your function is supposed to do. bocce
is the name of the game. What does this function do? Calculates the winner for a game of bocce. Cartesian point makes it clear what the type of jack
should be. Balls is holding the results for one frame in a game of bocce.
auto distance = std::sqrt(std::pow(jack[0], 2)+std::pow(jack[1], 2));
Use const
to define objects with values that do not change after construction. const
is useful for providing an immutable view of mutable data. This allows you to clarify to readers that the variable will not be modified and it prevents surprises when someone unexpectedly changes the objects value.
Also, distance from an origin exists in <cmath>
. See std::hypot
.
int_vec red = balls["red"];
int_vec blue = balls["blue"];
Key-access through a map returns a reference to the mapped value. Here, you've opted into making a copy of the mapped value. If you look back at your function signature, bocce_balls
is passed by value. Another copy. I'd advise that you pass the parameter by reference-to-const
as to prevent accidental modifications to the map. Keep the copies of red
and blue
here as you'll need those copies for the sort.
for (int i = 0; i < blue.size(); ++i)
if (red[i] > distance && red[i] > blue[i])
++red_count;
else if (blue[i] > distance && blue[i] > red[i])
++blue_count;
i
is a signed integer. blue.size()
returns an unsigned integer. Each loops does a signed/unsigned comparison. Every time you index with i
, the subscript operator is expecting an unsigned integer which leads to implicit conversions that change signedness. Turn up your warning levels.
$endgroup$
$begingroup$
Although it is against the rules to thank people, I shall thank you for taking your time to correct my code. So thanks.
$endgroup$
– ChubakBidpaa
Nov 16 '18 at 10:26
add a comment |
$begingroup$
typedef std::map<std::string, std::vector<int>> bocce_balls;
Are you interested in the ordering of the container? There really is no need to use std::set
to wrap the ball distances for each player. std::unordered_map
would be faster (consumes more space!), but do you even need a map-type? Have you considered using an array of vectors to represent the distances for each player? Do you need to wrap the distances or can you just pass each player as arguments?
Should the key be a string or an enumeration? Strings are great when you need to make keys on the fly at run-time. At compile-time, spelling errors with strings will result in new entries being created in mutable maps. With strongly typed enumerations, the possible range of keys is predefined.
Should distances be stored as integers or as a floating-point type?
std::string bocce(bocce_balls balls, int_arr jack)
Pick descriptive names that help readers understand what your function is supposed to do. bocce
is the name of the game. What does this function do? Calculates the winner for a game of bocce. Cartesian point makes it clear what the type of jack
should be. Balls is holding the results for one frame in a game of bocce.
auto distance = std::sqrt(std::pow(jack[0], 2)+std::pow(jack[1], 2));
Use const
to define objects with values that do not change after construction. const
is useful for providing an immutable view of mutable data. This allows you to clarify to readers that the variable will not be modified and it prevents surprises when someone unexpectedly changes the objects value.
Also, distance from an origin exists in <cmath>
. See std::hypot
.
int_vec red = balls["red"];
int_vec blue = balls["blue"];
Key-access through a map returns a reference to the mapped value. Here, you've opted into making a copy of the mapped value. If you look back at your function signature, bocce_balls
is passed by value. Another copy. I'd advise that you pass the parameter by reference-to-const
as to prevent accidental modifications to the map. Keep the copies of red
and blue
here as you'll need those copies for the sort.
for (int i = 0; i < blue.size(); ++i)
if (red[i] > distance && red[i] > blue[i])
++red_count;
else if (blue[i] > distance && blue[i] > red[i])
++blue_count;
i
is a signed integer. blue.size()
returns an unsigned integer. Each loops does a signed/unsigned comparison. Every time you index with i
, the subscript operator is expecting an unsigned integer which leads to implicit conversions that change signedness. Turn up your warning levels.
$endgroup$
typedef std::map<std::string, std::vector<int>> bocce_balls;
Are you interested in the ordering of the container? There really is no need to use std::set
to wrap the ball distances for each player. std::unordered_map
would be faster (consumes more space!), but do you even need a map-type? Have you considered using an array of vectors to represent the distances for each player? Do you need to wrap the distances or can you just pass each player as arguments?
Should the key be a string or an enumeration? Strings are great when you need to make keys on the fly at run-time. At compile-time, spelling errors with strings will result in new entries being created in mutable maps. With strongly typed enumerations, the possible range of keys is predefined.
Should distances be stored as integers or as a floating-point type?
std::string bocce(bocce_balls balls, int_arr jack)
Pick descriptive names that help readers understand what your function is supposed to do. bocce
is the name of the game. What does this function do? Calculates the winner for a game of bocce. Cartesian point makes it clear what the type of jack
should be. Balls is holding the results for one frame in a game of bocce.
auto distance = std::sqrt(std::pow(jack[0], 2)+std::pow(jack[1], 2));
Use const
to define objects with values that do not change after construction. const
is useful for providing an immutable view of mutable data. This allows you to clarify to readers that the variable will not be modified and it prevents surprises when someone unexpectedly changes the objects value.
Also, distance from an origin exists in <cmath>
. See std::hypot
.
int_vec red = balls["red"];
int_vec blue = balls["blue"];
Key-access through a map returns a reference to the mapped value. Here, you've opted into making a copy of the mapped value. If you look back at your function signature, bocce_balls
is passed by value. Another copy. I'd advise that you pass the parameter by reference-to-const
as to prevent accidental modifications to the map. Keep the copies of red
and blue
here as you'll need those copies for the sort.
for (int i = 0; i < blue.size(); ++i)
if (red[i] > distance && red[i] > blue[i])
++red_count;
else if (blue[i] > distance && blue[i] > red[i])
++blue_count;
i
is a signed integer. blue.size()
returns an unsigned integer. Each loops does a signed/unsigned comparison. Every time you index with i
, the subscript operator is expecting an unsigned integer which leads to implicit conversions that change signedness. Turn up your warning levels.
edited Nov 16 '18 at 10:44
answered Nov 16 '18 at 10:18
SnowhawkSnowhawk
5,56611229
5,56611229
$begingroup$
Although it is against the rules to thank people, I shall thank you for taking your time to correct my code. So thanks.
$endgroup$
– ChubakBidpaa
Nov 16 '18 at 10:26
add a comment |
$begingroup$
Although it is against the rules to thank people, I shall thank you for taking your time to correct my code. So thanks.
$endgroup$
– ChubakBidpaa
Nov 16 '18 at 10:26
$begingroup$
Although it is against the rules to thank people, I shall thank you for taking your time to correct my code. So thanks.
$endgroup$
– ChubakBidpaa
Nov 16 '18 at 10:26
$begingroup$
Although it is against the rules to thank people, I shall thank you for taking your time to correct my code. So thanks.
$endgroup$
– ChubakBidpaa
Nov 16 '18 at 10:26
add a comment |
Thanks for contributing an answer to Code Review Stack Exchange!
- Please be sure to answer the question. Provide details and share your research!
But avoid …
- Asking for help, clarification, or responding to other answers.
- Making statements based on opinion; back them up with references or personal experience.
Use MathJax to format equations. MathJax reference.
To learn more, see our tips on writing great answers.
Sign up or log in
StackExchange.ready(function ()
StackExchange.helpers.onClickDraftSave('#login-link');
);
Sign up using Google
Sign up using Facebook
Sign up using Email and Password
Post as a guest
Required, but never shown
StackExchange.ready(
function ()
StackExchange.openid.initPostLogin('.new-post-login', 'https%3a%2f%2fcodereview.stackexchange.com%2fquestions%2f207781%2fa-game-called-bocce-played-on-a-cartesian-grid%23new-answer', 'question_page');
);
Post as a guest
Required, but never shown
Sign up or log in
StackExchange.ready(function ()
StackExchange.helpers.onClickDraftSave('#login-link');
);
Sign up using Google
Sign up using Facebook
Sign up using Email and Password
Post as a guest
Required, but never shown
Sign up or log in
StackExchange.ready(function ()
StackExchange.helpers.onClickDraftSave('#login-link');
);
Sign up using Google
Sign up using Facebook
Sign up using Email and Password
Post as a guest
Required, but never shown
Sign up or log in
StackExchange.ready(function ()
StackExchange.helpers.onClickDraftSave('#login-link');
);
Sign up using Google
Sign up using Facebook
Sign up using Email and Password
Sign up using Google
Sign up using Facebook
Sign up using Email and Password
Post as a guest
Required, but never shown
Required, but never shown
Required, but never shown
Required, but never shown
Required, but never shown
Required, but never shown
Required, but never shown
Required, but never shown
Required, but never shown
1
$begingroup$
Don't use std::pow for squaring, you can hardly be less efficient... Just multiply the value with itself instead.
$endgroup$
– Aconcagua
Nov 16 '18 at 9:08