Skip to content

Conversation

@NikczemnyOkularnik
Copy link

@NikczemnyOkularnik NikczemnyOkularnik commented Aug 15, 2021

Copy link
Contributor

@ziobron ziobron left a comment

Choose a reason for hiding this comment

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

Just checking automation

@ziobron
Copy link
Contributor

ziobron commented Sep 10, 2021

Thanks @ziobron for Code Review.
🏅 1 XP granted

Copy link
Contributor

@ziobron ziobron left a comment

Choose a reason for hiding this comment

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

Quick additional check

Copy link
Contributor

@ziobron ziobron left a comment

Choose a reason for hiding this comment

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

And the last check

BarTes8 pushed a commit to BarTes8/object-oriented-programming that referenced this pull request Sep 18, 2021
Comment on lines +87 to +88
std::cin.clear();
std::cin.ignore(std::numeric_limits<std::streamsize>::max(), '\n');
Copy link
Collaborator

Choose a reason for hiding this comment

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

This is repeated few times, maybe create function clearInputStream()?
DRY rule -> don't repeat yourself :)

Comment on lines +46 to +48
Cargo* Alcohol::clone()
{
return new Alcohol(*this);
Copy link
Collaborator

Choose a reason for hiding this comment

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

Avoid raw ptr! and new operator

return std::unique_ptr and use std::make_unique.

shm/fruit.cpp Outdated
size_t Fruit::getPrice() const
{
return static_cast<size_t>(basePrice_ * ((size_t)(expiry_date_ - time_elapsed_)) / expiry_date_);
//return static_cast<size_t>(basePrice_ * ((size_t)(expiry_date_ - time_elapsed_)) / expiry_date_);
Copy link
Collaborator

Choose a reason for hiding this comment

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

remove commented code.
This commented code looks good, why do you return basePrice_ instead?

Comment on lines +61 to +63
Cargo *Fruit::clone()
{
return new Fruit(*this);
Copy link
Collaborator

Choose a reason for hiding this comment

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

unique_ptr

shm/item.cpp Outdated
Comment on lines 49 to 51
Cargo* Item::clone()
{
return new Item(*this);
Copy link
Collaborator

Choose a reason for hiding this comment

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

unique_ptr

shm/ship.cpp Outdated
Comment on lines 53 to 54
auto cargoPtr = findMatchCargo(item);
if(findMatchCargo(item))
if(cargoPtr)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Better

if (cargoPtr = findMatchCargo(item)) {

}

If automatically check if this is not nullptr

shm/ship.cpp Outdated
Comment on lines 95 to 105
//Cargo * toAdd;
loadCargo->reduceAmount(amount);
Cargo* toAdd;
if(Fruit* f = dynamic_cast<Fruit*>(loadCargo)){
toAdd = new Fruit(*f);

}
if(Item* i = dynamic_cast<Item*>(loadCargo)){
toAdd = new Item(*i);
}
if(Alcohol* a = dynamic_cast<Alcohol*>(loadCargo)){
toAdd = new Alcohol(*a);
}

// if(Fruit* f = dynamic_cast<Fruit*>(loadCargo)){
// toAdd = new Fruit(*f);
// }
// if(Item* i = dynamic_cast<Item*>(loadCargo)){
// toAdd = new Item(*i);
// }
// if(Alcohol* a = dynamic_cast<Alcohol*>(loadCargo)){
// toAdd = new Alcohol(*a);
// }
Copy link
Collaborator

Choose a reason for hiding this comment

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

Remove commented code

NikczemnyOkularnik and others added 3 commits January 15, 2022 15:31
We do not use gtest
Tests in catch2
Comment on lines +35 to +36
int x = (int)distrib(gen);
int y = (int)distrib(gen);
Copy link
Collaborator

Choose a reason for hiding this comment

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

we should avoid old (C) style casting. Because it unsafe

{
Coordinates c (static_cast<float>(distrib(gen)), static_cast<float>(distrib(gen)));

for (int i = 0; i < (int)constVariables::ISLANDS_COUNT; i++)
Copy link
Collaborator

Choose a reason for hiding this comment

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

better size_t i = 0 and don't cast to int

Comment on lines +40 to +41
x = (int)distrib(gen);
y = (int)distrib(gen);
Copy link
Collaborator

Choose a reason for hiding this comment

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

avoid old (C) style casting

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

8 participants