Shm 2 #106
Open
Shm 2 #106
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Co-authored-by: Kacu <[email protected]>
#11) * 6 moved definitions from Fortune.hpp file into Fortune.cpp source file * 6 changed Fortune class into namespace Co-authored-by: Kacu <[email protected]>
Co-authored-by: Kacu <[email protected]>
Co-authored-by: Kacu <[email protected]>
Co-authored-by: Kacu <[email protected]>
Co-authored-by: Kacu <[email protected]>
* 20 created new class Fruit * 20 created new class Alcohol * 20 created new class Item * 20 turned Cargo class into interface * 20 created load and unload operations inside Ship class Co-authored-by: Kacu <[email protected]>
* 14 created Store class * 14 created new classes Time and Observer * 14 connected observer with five classes Co-authored-by: Kacu <[email protected]>
* 13 created new class Delegate * 13 implemented nextDay operations inside Ship and Store classes Co-authored-by: Kacu <[email protected]>
Co-authored-by: Kacu <[email protected]>
Co-authored-by: Kacu <[email protected]>
nauka-programowania-MA
requested changes
Aug 23, 2021
|
|
||
| class Delegate { | ||
| public: | ||
| // ~Delegate(); |
There was a problem hiding this comment.
You should have virtual D'tor when one or more methods are virtual
Comment on lines
+3
to
+4
| std::random_device randomDevice; | ||
| std::mt19937 randomEngine(randomDevice()); |
There was a problem hiding this comment.
Don't make it global, move them to class
Comment on lines
+10
to
+21
| case Rarity::Common: | ||
| return getBasePrice() / 3; | ||
|
|
||
| case Rarity::Rare: | ||
| return getBasePrice() / 2; | ||
|
|
||
| case Rarity::Epic: | ||
| return getBasePrice(); | ||
|
|
||
| case Rarity::Legendary: | ||
| return getBasePrice() * 4; | ||
| } |
There was a problem hiding this comment.
better to always multiply. So common should be 1, rare eg. 3 etc.. Dividing always makes trouble in testing, because you lose precision.
Comment on lines
+5
to
+6
| horizonLimit_(100), | ||
| islands_(generateIslands(10)), |
There was a problem hiding this comment.
Make this 2 magic numbers constexpr
| std::vector<Island> islands; | ||
| islands.reserve(xyPositions.size() / 2); | ||
| for (size_t i = 1; i < xyPositions.size(); i += 2) { | ||
| islands.emplace_back(Island::Coordinates(xyPositions.at(i - 1), xyPositions.at(i))); |
There was a problem hiding this comment.
when you use ecmplae_back you provide arguments immediately to C'tor, if you want to create an object and insert it better use push_back.
like vec.empalce_back(5, "ala");
There was a problem hiding this comment.
In other cases, you use copy or move C'tor...
Comment on lines
+13
to
+25
| Done, | ||
| Lack_of_money, | ||
| Lack_of_cargo, | ||
| Lack_of_space | ||
| }; | ||
|
|
||
| Time* time_; | ||
| std::vector<std::unique_ptr<Cargo>> cargo_; | ||
|
|
||
| public: | ||
| // Store(Time* time); | ||
|
|
||
| // ~Store() override; |
There was a problem hiding this comment.
You should make it default and overrdie
Comment on lines
+23
to
+25
| // Store(Time* time); | ||
|
|
||
| // ~Store() override; |
Comment on lines
+29
to
+41
| // Cargo* GetCargo(const size_t pos); | ||
|
|
||
| // Response Buy(Cargo* cargo, size_t amount, Player* player); | ||
|
|
||
| // Response Sell(Cargo* caergo, size_t amount, Player* player); | ||
|
|
||
| // friend std::ostream& operator<<(std::ostream& out, const Store& store); | ||
|
|
||
| private: | ||
| // void GenerateCargo(); | ||
|
|
||
| // Cargo* FindMatchCargo(Cargo* cargo); | ||
|
|
| return *this; | ||
| } | ||
|
|
||
| // ObserverIT GetObserverIt(Observer* obs); |
| struct Observer { | ||
| virtual void nextDay() = 0; | ||
|
|
||
| // virtual ~Observer(); |
There was a problem hiding this comment.
should be virtual
BarTes8
pushed a commit
to BarTes8/object-oriented-programming
that referenced
this pull request
Sep 18, 2021
Exit function and confirm
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
No description provided.