Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Shm #110

Open
wants to merge 113 commits into
base: shm
Choose a base branch
from
Open

Shm #110

wants to merge 113 commits into from

Conversation

bartosz-gruszczyk
Copy link

ziobron and others added 30 commits June 11, 2021 19:12
Copy link

@lukasz-kukulka lukasz-kukulka left a comment

Choose a reason for hiding this comment

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

You mixed formatting style with brackets, sometimes you use:
function() {

}
sometimes:
function()
{

}

shm/cargo.cpp Outdated
Comment on lines 24 to 26
} else {

}

Choose a reason for hiding this comment

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

Redundant else

shm/game.cpp Outdated
Comment on lines 156 to 220
void Game::sell()
{
std::cout << "Your are selling. Remember that selling will take one day!\n";
if (!map_.getIsland(player_->getPlayerPosition()))
{
std::cout << "Captain, we are on the sea, you can not sell enything!\n";
return;
}
if (player_->getAvailableSpace() == CAPACITY)
{
std::cout << "Captain, we have nothing to sell!\n";
return;
}
std::cout << "What you want to sell?" << '\n';
player_->printCargo();
std::cout << "At store prices: \n";
auto actualStoreVector = map_.getIsland(player_->getPlayerPosition())->getStore()->getCargoOfStore();
std::vector<std::shared_ptr<Cargo>>::iterator atStore;
for (auto object : ship_->getCargosVector())
{
atStore = std::find_if(actualStoreVector.begin(), actualStoreVector.end(), [&object](const auto &atStore)
{ return object->getName() == atStore->getName(); });
if (atStore != actualStoreVector.end())
{
std::cout << "Name: " << (*atStore)->getName() << ", Price: " << (*atStore)->getPrice() << '\n';
}
}
std::cout << ship_->getCargosVector().size() + 1 << " Exit\n";
int sellOption = 0;
std::cin >> sellOption;
while (ship_->getCargosVector().size() < sellOption)
{
if (sellOption == ship_->getCargosVector().size() + 1)
{
std::cout << "Enough trade for today.\n";
return;
}
std::cout << "Segmentation fault Captain, you have to change your decision!\n";
std::cin >> sellOption;
}
auto objectToSell = ship_->getCargosVector()[sellOption - 1];
std::cout << "Amount of " << objectToSell->getName() << " to sell:\n";
int amountOfCargoToSell = 0;
std::cin >> amountOfCargoToSell;
while (objectToSell->getAmount() < amountOfCargoToSell)
{
std::cout << "Captain, we don't have so many of " << objectToSell->getName() << " What we do now?\n1. Change amount\n2. Exit" << '\n';
std::cin >> sellOption;
if (sellOption == 1)
{
std::cout << "Amount of " << objectToSell->getName() << " to sell:\n";
std::cin >> amountOfCargoToSell;
}
else
{
return;
}
}
atStore = std::find_if(actualStoreVector.begin(), actualStoreVector.end(), [&objectToSell](const auto &atStore)
{ return objectToSell->getName() == atStore->getName(); });
ship_->unload(objectToSell, amountOfCargoToSell);
player_->setMoney(player_->getMoney() + (*atStore)->getPrice() * amountOfCargoToSell);
map_.getIsland(player_->getPlayerPosition())->getStore()->addCargo(objectToSell, amountOfCargoToSell);
time_->onTimeChange();
}

Choose a reason for hiding this comment

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

To long function, you should split this for more

shm/game.cpp Outdated
Comment on lines 237 to 278
void Game::travel()
{
std::cout << "Where you want to travel?\n";
int islandCounter = 0;
std::vector<int> daysToGo;
daysToGo.reserve(map_.getEveryIsland().size());
for (auto island : map_.getEveryIsland())
{
int amountOfDays = island.getPosition().distance(player_->getPlayerPosition()) / ship_->getSpeed();
if (amountOfDays == 0 && !(island.getPosition() == player_->getPlayerPosition()))
{
daysToGo.push_back(1);
}
else
{
daysToGo.push_back(amountOfDays);
}
islandCounter++;
std::cout << "Island " << islandCounter << " Travel time: " << daysToGo[islandCounter - 1] << " days\n";
}
std::cout << ++islandCounter << ". Exit\n";
std::cout << "Where are we going captain?: ";
islandCounter = 0;
std::cin.clear();
std::cin >> islandCounter;
while (islandCounter > map_.getEveryIsland().size() + 1)
{
std::cout << "Captain you have old map! Here is new one, so where we are going?: \n";
std::cin.clear();
std::cin >> islandCounter;
}
if (islandCounter == map_.getEveryIsland().size() + 1)
{
std::cout << "I changed my mind, we stay here!\n";
return;
}
player_->setPlayerPosition(map_.getEveryIsland()[islandCounter - 1].getPosition().getPositionX(), map_.getEveryIsland()[islandCounter - 1].getPosition().getPositionY());
for (int i = 0; i < daysToGo[islandCounter - 1]; i++)
{
time_->onTimeChange();
}
}

Choose a reason for hiding this comment

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

The same as sell function

shm/game.cpp Outdated
Comment on lines 280 to 374
void Game::buy()
{
auto IslandWeAreOn = map_.getIsland(player_->getPlayerPosition());
if (IslandWeAreOn)
{
std::cout << "Buy. Remember that buying will take one day!\n";
std::cout.flush();
std::cout << *IslandWeAreOn->getStore() << '\n';
std::random_device rm;
std::mt19937 gr(rm());
int EasterEggPossibilities = 20;
std::uniform_int_distribution<> EasterEgg(0, EasterEggPossibilities);
int whahappen = EasterEgg(gr);
if (whahappen == 10)
{
std::cout << IslandWeAreOn->getStore()->getCargoOfStore().size() + 2 << ". Maybe round of Gwent?\n";
}
std::cout << "What do you want to buy: \n";
int storeIndex = 0;
std::cin.clear();
std::cin >> storeIndex;
while (storeIndex > IslandWeAreOn->getStore()->getCargoOfStore().size())
{
if (storeIndex == IslandWeAreOn->getStore()->getCargoOfStore().size() + 1)
{
std::cout << "Get back on the ship, you scums!\n";
return;
}
if (whahappen == 10 && storeIndex == IslandWeAreOn->getStore()->getCargoOfStore().size() + 2)
{
std::cout << "Winds howling... (O'Dimm theme starts playing...)\n";
return;
}
std::cout << "Too much rum, captain? Choose one more time: ";
std::cin.clear();
std::cin >> storeIndex;
}
auto CargoWeAreBuying = IslandWeAreOn->getStore()->getCargoOfStore()[storeIndex - 1];
size_t amountOfCargo = CargoWeAreBuying->getAmount();
std::cout << "Amount (from 1 to " << amountOfCargo << "): \n";
int amountOfCargoToBuy = 0;
std::cin.clear();
std::cin >> amountOfCargoToBuy;
if (amountOfCargoToBuy > amountOfCargo)
{
std::cout << "Captain, they didn't have " << amountOfCargoToBuy << " of " << CargoWeAreBuying->getName() << ". We bought only " << amountOfCargo << "\n";
amountOfCargoToBuy = amountOfCargo;
}
int choose = 0;
if (CargoWeAreBuying->getPrice() * static_cast<size_t>(amountOfCargoToBuy) > player_->getMoney())
{
std::cout << "Captain, we do not have enough money! We can only buy " << player_->getMoney() / CargoWeAreBuying->getPrice() << " of " << CargoWeAreBuying->getName();
std::cout << ". What we do now? \n1. Buy other amount of " << CargoWeAreBuying->getName() << "\n2. Exit\n";
std::cin.clear();
std::cin >> choose;
if (choose == 1)
{
std::cout << "Amount (from 1 to " << player_->getMoney() / CargoWeAreBuying->getPrice() << "): \n";
std::cin.clear();
std::cin >> amountOfCargoToBuy;
}
else
{
return;
}
}
while (player_->getAvailableSpace() - amountOfCargoToBuy < 0)
{
std::cout << "Captain, our cargo is full, we can only buy " << player_->getAvailableSpace() << " of " << CargoWeAreBuying->getName() << ". What we are doing now?\n";
std::cout << "1. Set other value\n2. Exit\n";
std::cin.clear();
std::cin >> choose;
if (choose == 1)
{
std::cout << "New value: \n";
std::cin.clear();
std::cin >> amountOfCargoToBuy;
}
else
{
return;
}
}
ship_->load(CargoWeAreBuying, amountOfCargoToBuy);
IslandWeAreOn->getStore()->loadShip(CargoWeAreBuying, amountOfCargoToBuy);
player_->setMoney(player_->getMoney() - CargoWeAreBuying->getPrice() * static_cast<size_t>(amountOfCargoToBuy) < 0 ? 0 : player_->getMoney() - CargoWeAreBuying->getPrice() * static_cast<size_t>(amountOfCargoToBuy));
std::cout << player_->getMoney() << '\n';
player_->countAvailableSpace();
time_->onTimeChange();
}
else
{
std::cout << "Captain, we are not on any island!\n";
}
}

Choose a reason for hiding this comment

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

The same as sell function

Exit
};


Choose a reason for hiding this comment

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

Redundant line

void Ship::load(std::shared_ptr<Cargo> &cargo, size_t amount)
{
auto findCargo = findMatchCargo(cargo);

Choose a reason for hiding this comment

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

Redundant line

shm/ship.cpp Outdated

if (findCargo != cargos_.end())
{

Choose a reason for hiding this comment

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

Redundant line

shm/ship.cpp Outdated
}
else
{

Choose a reason for hiding this comment

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

Redundant line

Comment on lines +159 to +160
}
void Ship::nextDay()

Choose a reason for hiding this comment

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

You need to separate this functions

Comment on lines +28 to +29


Choose a reason for hiding this comment

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

Redundant lines

@ziobron
Copy link
Contributor

ziobron commented Sep 14, 2021

Thanks @lukasz-kukulka for Code Review.
🏅 1 XP granted

shm/alcohol.cpp Outdated
Comment on lines 11 to 14
Cargo& Alcohol::operator+=(size_t amount) {
amount_ += amount;
return *this;
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

What happend when you add to much cargo (more then capacity?) Do you handle this somewhere? If yes please move also below logic to this function (with substract) because now you have logic for substract in Cargo and for add in other class this is not god :)

shm/alcohol.hpp Outdated
class Alcohol : public Cargo {
public:
Alcohol(const std::string& name, size_t amount, size_t basePrice, size_t percentage);
Alcohol() {};
Copy link
Collaborator

Choose a reason for hiding this comment

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

= default

shm/alcohol.hpp Outdated
public:
Alcohol(const std::string& name, size_t amount, size_t basePrice, size_t percentage);
Alcohol() {};
Alcohol(const std::string& name, size_t amount) : Cargo(name, amount) {};
Copy link
Collaborator

Choose a reason for hiding this comment

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

to cpp

shm/alcohol.hpp Outdated
Alcohol(const std::string& name, size_t amount, size_t basePrice, size_t percentage);
Alcohol() {};
Alcohol(const std::string& name, size_t amount) : Cargo(name, amount) {};
~Alcohol() override{};
Copy link
Collaborator

Choose a reason for hiding this comment

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

= default

, time_(time)
{
if (time_) {
time_->attachObserver(this);
Copy link
Collaborator

Choose a reason for hiding this comment

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

You never detach observer. This will cause crash when this object will be deleted

shm/store.cpp Outdated


std::vector<std::shared_ptr<Cargo>>::iterator Store::findMatchCargo(const std::shared_ptr<Cargo> wantedCargo) {
auto find = std::find_if(stock_.begin(),
Copy link
Collaborator

Choose a reason for hiding this comment

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

return

shm/store.cpp Outdated
stock_.end(),
[&wantedCargo](const auto& cargo) { return cargo->getName() == wantedCargo->getName(); });

return find;
Copy link
Collaborator

Choose a reason for hiding this comment

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

redundant

Comment on lines +48 to +49
auto shipCargo = playerShip->findMatchCargo(cargo);
auto shipStock = playerShip->getCargosVector();
Copy link
Collaborator

Choose a reason for hiding this comment

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

const

shm/store.cpp Outdated
Comment on lines 172 to 188
if (Alcohol* alcohol = dynamic_cast<Alcohol*>(cargo.get())) {
stock_.push_back(std::make_shared<Alcohol>(alcohol->getName(),
amount,
alcohol->getBasePrice(),
alcohol->getPercentage()));
} else if (Fruit* fruit = dynamic_cast<Fruit*>(cargo.get())) {
stock_.push_back(std::make_shared<Fruit>(fruit->getName(),
amount,
fruit->getBasePrice(),
fruit->getExpirationDate(),
nullptr));
} else if (Item* item = dynamic_cast<Item*>(cargo.get())) {
stock_.push_back(std::make_shared<Item>(item->getName(),
amount,
item->getPrice(),
item->getRarity()));
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

use clone method

Comment on lines +196 to +199
int more_stuff_in_store = 1;
int less_stuff_in_store = 0;
int adding_taking_max = 10;
int adding_taking_min = 1;
Copy link
Collaborator

Choose a reason for hiding this comment

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

const

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants