-
Notifications
You must be signed in to change notification settings - Fork 3
Obstacle and CircularObstacle #12
base: master
Are you sure you want to change the base?
Conversation
We should write some unit tests before merging the branch |
float CircularObstacle::squaredDistance(const Vector2D &pos) const | ||
{ | ||
// TODO : is "distance" really necessary ? (it uses a std::sqrt computation) | ||
float out = std::max(0.f, pos.distance(rotation_center_) - radius_); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should consider implementing an approximation of the euclidean distance that does not uses sqrt and add an entry in the config for this. (this has nothing to do with the merging request)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
auto should be used in a lot of places here, I didn't mention everything. I probably missed some constexpr and noexcept too.
Can we force operator&& on std::vector<Obstacle>
in CompoudObstacle ?
if (typeid(*this) != typeid(rhs)) | ||
return false; | ||
|
||
return static_cast<const CompoundObstacle &>(rhs).obstacles_list_ == obstacles_list_; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same as CircularObstacle::operator==(const Obstacle &rhs) const
if (typeid(*this) != typeid(rhs)) | ||
return false; | ||
|
||
return radius_ == static_cast<const CircularObstacle &>(rhs).radius_; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Shouldn't this be factorised? Something like:
return Obstacle::operator==(rhs) && typeid(*this) != typeid(rhs)
&& radius_ == static_cast<const CircularObstacle &>(rhs).radius_;
|
||
Vector2D RectangularObstacle::toObstacleCoordinateSystem(const Vector2D &point) const | ||
{ | ||
return Vector2D(getXToObstacleCoordinateSystem(point), getYToObstacleCoordinateSystem(point)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The return type is already specified as Vector2D, a braced initializer is enough here.
return { getXToObstacleCoordinateSystem(point), getYToObstacleCoordinateSystem(point) };
|
||
Vector2D RectangularObstacle::toTableCoordinateSystem(const Vector2D &point) const | ||
{ | ||
return Vector2D(cos_ * point.getX() - sin_ * point.getY() + rotation_center_.getX(), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same as RectangularObstacle::toObstacleCoordinateSystem(const Vector2D &point) const
Vector2D pointA = corners[i]; | ||
Vector2D pointB = corners[(i + 1) % 4]; | ||
float distance = pointA.distance(pointB); | ||
int nbPoints = static_cast<int>(std::ceil(distance / longestAllowedLength)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
auto can be used here because the types are obvious:
auto distance
auto nbPoints
|
||
float RectangularObstacle::squaredDistance(const Vector2D &v) const | ||
{ | ||
Vector2D in = toObstacleCoordinateSystem(v); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can be const auto in
(but with a better name than in ?)
|
||
bool RectangularObstacle::isInObstacle(const Vector2D &pos) const | ||
{ | ||
Vector2D in = toObstacleCoordinateSystem(pos); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can be const auto in
(but with a better name than in ?)
if (typeid(*this) != typeid(rhs)) | ||
return false; | ||
|
||
RectangularObstacle ro_rhs = static_cast<const RectangularObstacle &>(rhs); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
const auto again
right_upper_corner_ == ro_rhs.right_upper_corner_; | ||
} | ||
|
||
bool RectangularObstacle::test_separation(const float &a, const float &b, const float &a2, const float &b2, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
RectangularObstacle::test_separation
can have the noexcept and constexpr specifiers
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
std::min and std::max are not declared with noexcept... So i don't know if it's really a good idea.
In C++11: " constexpr functions may contain no more than a single executable statement: a return" Effective Modern C++ Scott Meyers, p99, but, i'm fine with C++14 ;-)
obs.getYToObstacleCoordinateSystem(right_upper_corner_rotate_)); | ||
} | ||
|
||
float RectangularObstacle::getHalfDiagonal() const |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
RectangularObstacle::getHalfDiagonal
can be declared with noexcept
Issue #2
Method
bool isColliding(RectangularObstacle obs)
is missing becauseRectangularObstacle
is missing.