Skip to content

Commit

Permalink
Remove some magic numbers uses.
Browse files Browse the repository at this point in the history
  • Loading branch information
przemek83 committed Sep 15, 2024
1 parent a2ca0c8 commit d968562
Show file tree
Hide file tree
Showing 13 changed files with 65 additions and 35 deletions.
2 changes: 2 additions & 0 deletions CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,8 @@ set(${PROJECT_NAME}_SOURCES
src/UserChoice.h
src/Display.h
src/Display.cpp
src/Utils.h
src/Utils.cpp
src/map/Water.cpp
src/map/Water.h
src/map/Steel.cpp
Expand Down
11 changes: 7 additions & 4 deletions src/Bullet.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -4,12 +4,14 @@
#include "Direction.h"
#include "PointUtils.h"
#include "Screen.h"
#include "Utils.h"

Bullet::Bullet(Point startingPoint, int speed, bool playerOrigin, int power,
Direction direction)
: Drawable(
{startingPoint.x_ - (Config::getInstance().getBulletSize() / 2),
startingPoint.y_ - (Config::getInstance().getBulletSize() / 2)}),
: Drawable({startingPoint.x_ -
(utils::getMidpoint(Config::getInstance().getBulletSize())),
startingPoint.y_ -
utils::getMidpoint(Config::getInstance().getBulletSize())}),
size_(Config::getInstance().getBulletSize()),
playerOrigin_(playerOrigin),
direction_(direction),
Expand All @@ -24,7 +26,8 @@ int Bullet::getPower() const { return power_; }

Point Bullet::getCenter() const
{
return {getX() + (size_ / 2), getY() + (size_ / 2)};
return {getX() + utils::getMidpoint(size_),
getY() + utils::getMidpoint(size_)};
}

int Bullet::getDirectionX() const
Expand Down
7 changes: 4 additions & 3 deletions src/Display.cpp
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
#include "Display.h"

#include "Config.h"
#include "Utils.h"

Display::Display()
: width_(Config::getInstance().getBoardWidth() +
Expand All @@ -9,10 +10,10 @@ Display::Display()
{
}

int Display::getCenterX() const { return width_ / 2; }
int Display::getCenterY() const { return height_ / 2; }
int Display::getCenterX() const { return utils::getMidpoint(width_); }
int Display::getCenterY() const { return utils::getMidpoint(height_); }

int Display::getWidth() const { return width_; }
void Display::setWidth(int width) { width_ = width; }
int Display::getHeight() const { return height_; }
void Display::setHeight(int height) { height_ = height; }
void Display::setHeight(int height) { height_ = height; }
9 changes: 5 additions & 4 deletions src/Game.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
#include "PointUtils.h"
#include "Screen.h"
#include "Tank.h"
#include "Utils.h"

Game::Game(Screen& screen)
: status_({Config::getInstance().getBoardWidth(), 0}),
Expand Down Expand Up @@ -154,10 +155,10 @@ void Game::control(Map& map, std::list<Tank>& tanks, std::list<Bullet>& bullets)
void Game::drawEndOfGame(const std::string& text) const
{
Screen::clearScreenWithBlack();
screen_.drawText((Config::getInstance().getBoardWidth() +
Config::getInstance().getSatusWidth()) /
2,
Config::getInstance().getBoardHeight() / 2, text);
screen_.drawText(utils::getMidpoint(Config::getInstance().getBoardWidth() +
Config::getInstance().getSatusWidth()),
utils::getMidpoint(Config::getInstance().getBoardHeight()),
text);
Screen::refresh();
std::this_thread::sleep_for(std::chrono::seconds(2));
}
Expand Down
5 changes: 3 additions & 2 deletions src/Menu.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
#include "MenuItem.h"
#include "Screen.h"
#include "UserChoice.h"
#include "Utils.h"

Menu::Menu(Screen& screen) : screen_(screen) { initMainMenu(); }

Expand Down Expand Up @@ -141,8 +142,8 @@ bool Menu::mouseIsHoveringItem(std::pair<int, int> mousePosition,
const auto [mouseX, mouseY] = mousePosition;
const int itemWidth{items_.front().getWidth()};
const int itemHeight{items_.front().getHeight()};
return (mouseX > (screen_.getCenterX() - (itemWidth / 2))) &&
(mouseX < (screen_.getCenterX() + (itemWidth / 2))) &&
return (mouseX > (screen_.getCenterX() - utils::getMidpoint(itemWidth))) &&
(mouseX < (screen_.getCenterX() + utils::getMidpoint(itemWidth))) &&
(mouseY > (firstItemY + (itemHeight * item))) &&
(mouseY < (firstItemY + itemHeight + (itemHeight * item)));
}
Expand Down
12 changes: 4 additions & 8 deletions src/MenuItem.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -5,11 +5,7 @@
#include "Display.h"
#include "ResourceType.h"
#include "Screen.h"

namespace
{
int getMidpoint(int value) { return value / 2; }
} // namespace
#include "Utils.h"

MenuItem::MenuItem(UserChoice userChoice)
: Drawable({}), userChoice_{userChoice}
Expand All @@ -27,7 +23,7 @@ ResourceType MenuItem::getResourceType() const { return resourceType_; }

Point MenuItem::getCenter() const
{
return {getMidpoint(width_), getMidpoint(height_)};
return {utils::getMidpoint(width_), utils::getMidpoint(height_)};
}

void MenuItem::setSelected(bool selected)
Expand All @@ -45,10 +41,10 @@ void MenuItem::init(const Display& display, int position, int count)
height_ = std::max(display.getHeight() / 10,
display.getResourceHeight(ResourceType::MENU_ITEM));

setX(display.getCenterX() - getMidpoint(width_));
setX(display.getCenterX() - utils::getMidpoint(width_));

const int locationOfFirstItem{display.getCenterY() -
getMidpoint(count * height_)};
utils::getMidpoint(count * height_)};
setY(locationOfFirstItem + (height_ * position));
}

Expand Down
4 changes: 3 additions & 1 deletion src/Status.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

#include "Config.h"
#include "Screen.h"
#include "Utils.h"

Status::Status(Point point) : Drawable(point) {}

Expand Down Expand Up @@ -33,7 +34,8 @@ void Status::draw(const Screen& screen) const

Point Status::getCenter() const
{
return {getX() + getWeidth() / 2, getHeight() / 2};
return {getX() + utils::getMidpoint(getWeidth()),
utils::getMidpoint(getHeight())};
}

ResourceType Status::getResourceType() const
Expand Down
3 changes: 2 additions & 1 deletion src/Tank.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
#include "Direction.h"
#include "Screen.h"
#include "TankType.h"
#include "Utils.h"

Tank::Tank(TankType tankType, Point point)
: Drawable(point), initialX_(point.x_), initialY_(point.y_)
Expand All @@ -32,7 +33,7 @@ ResourceType Tank::getResourceType() const

Point Tank::getCenter() const
{
const int middle{Config::getInstance().getTileSize() / 2};
const int middle{utils::getMidpoint(Config::getInstance().getTileSize())};
return {getX() + middle, getY() + middle};
}

Expand Down
6 changes: 6 additions & 0 deletions src/Utils.cpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
#include "Utils.h"

namespace utils
{
int getMidpoint(int value) { return value / 2; }
} // namespace utils
6 changes: 6 additions & 0 deletions src/Utils.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
#pragma once

namespace utils
{
int getMidpoint(int value);
}; // namespace utils
21 changes: 14 additions & 7 deletions test/BulletTest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -4,17 +4,21 @@
#include <src/Bullet.h>
#include <src/Config.h>
#include <src/Direction.h>
#include <src/Utils.h>

namespace
{
const int bulletPower{4};
const int bulletSpeed{10};
const Point point{5, 5};
const bool enemyOrigin = false;
const bool playerOrigin = true;
const bool enemyOrigin{false};
const bool playerOrigin{true};

static Bullet getExampleBullet()
Bullet getExampleBullet()
{
return {point, bulletSpeed, enemyOrigin, bulletPower, Direction::UP};
}
}; // namespace

TEST_CASE("Bullet getters", "[bullet]")
{
Expand Down Expand Up @@ -48,9 +52,11 @@ TEST_CASE("Bullet coordinates", "[bullet]")
{
const Point currentPoint{bullet.getLocation()};
REQUIRE(currentPoint.x_ ==
point.x_ - Config::getInstance().getBulletSize() / 2);
point.x_ -
utils::getMidpoint(Config::getInstance().getBulletSize()));
REQUIRE(currentPoint.y_ ==
point.y_ - Config::getInstance().getBulletSize() / 2);
point.y_ -
utils::getMidpoint(Config::getInstance().getBulletSize()));
}
SECTION("center is correct")
{
Expand Down Expand Up @@ -107,7 +113,8 @@ TEST_CASE("Bullet moving", "[bullet]")
SECTION("bullet moving inside valid area")
{
using TestData = std::pair<Direction, Point>;
const int middle{Config::getInstance().getBoardWidth() / 2};
const int middle{
utils::getMidpoint(Config::getInstance().getBoardWidth())};
auto [direction, expectedPoint] = GENERATE_REF(
TestData{Direction::UP, Point{middle, middle - bulletSpeed}},
TestData{Direction::DOWN, Point{middle, middle + bulletSpeed}},
Expand Down Expand Up @@ -144,7 +151,7 @@ TEST_CASE("Bullet moving to invalid area", "[bullet]")
{
using TestData = std::pair<Direction, Point>;
const int nearEndOfMap{Config::getInstance().getBoardWidth() -
bulletSpeed / 2};
utils::getMidpoint(bulletSpeed)};
auto [direction, pointGenerated] =
GENERATE_REF(TestData{Direction::UP, point},
TestData{Direction::DOWN, Point{point.x_, nearEndOfMap}},
Expand Down
5 changes: 3 additions & 2 deletions test/MapTest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
#include <src/Map.h>
#include <src/Point.h>
#include <src/Tank.h>
#include <src/Utils.h>

// 0 - plain
// 1 - brick
Expand Down Expand Up @@ -184,9 +185,9 @@ TEST_CASE("Check hitting", "[map]")
const Point point{tileToPoint(1, 2)};
map.loadMap(stream);
REQUIRE(map.isBaseDestroyed() == false);
map.hit(point, testHitStrength / 2);
map.hit(point, utils::getMidpoint(testHitStrength));
REQUIRE(map.isBaseDestroyed() == false);
map.hit(point, testHitStrength / 2);
map.hit(point, utils::getMidpoint(testHitStrength));
REQUIRE(map.isBaseDestroyed() == true);
}
}
Expand Down
9 changes: 6 additions & 3 deletions test/TankTest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
#include <src/Bullet.h>
#include <src/Config.h>
#include <src/Tank.h>
#include <src/Utils.h>

TEST_CASE("Tank creation", "[tank]")
{
Expand Down Expand Up @@ -93,7 +94,8 @@ TEST_CASE("location related", "[tank]")

SECTION("get center")
{
const int halfOfTile{Config::getInstance().getTileSize() / 2};
const int halfOfTile{
utils::getMidpoint(Config::getInstance().getTileSize())};
const Tank tank(TankType::ENEMY_TIER_1, point);
REQUIRE(tank.getCenter() == Point{
100 + halfOfTile,
Expand Down Expand Up @@ -267,8 +269,9 @@ TEST_CASE("firing", "[tank]")
const int tileSize{Config::getInstance().getTileSize()};
Tank tank(TankType::PLAYER_TIER_1, point);
const Bullet bullet = tank.fire(TimePoint::max());
const Point expectedBulletCenter{point.x_ + tileSize / 2,
point.y_ + tileSize / 2};
const Point expectedBulletCenter{
point.x_ + utils::getMidpoint(tileSize),
point.y_ + utils::getMidpoint(tileSize)};
REQUIRE(bullet.getCenter() == expectedBulletCenter);
}

Expand Down

0 comments on commit d968562

Please sign in to comment.