Return to Answer

15. Instead of checking for stream failure (in Dungeon::select_room_to_move) using fail, use std::cin's operator bool: if (!std::cin) /fail/;`.

16. if (game_over == true) can be simplified to if (game_over).

17. The introduction text is not up-to-date. There are three pits and super bats, not two. Might want to use the actual variables instead of hardcoding text.

18. It's my understanding that neighbors cannot ever be nullptr, so the extra if statement for it is unnecessary (wumpus.cpp:226). Might want to consider using an assert instead.

19. It seems like you are using player_room a lot. You might want to consider using a member variable for that so that you avoid to iterate over (potentially) the whole room container.

20. Dungeon::shoot_arrow has a bug. If I want to shoot an arrow into 3-5-9, then I only lose one arrow. I don't think that this intended. Also, if I shoot my last arrow into a non-existent room, then I can illegally continue ;).

21. Dungeon::move_player has a bug, it moves the player into the wrong room. That's because rooms is not sorted, so rooms[target_room_number] doesn't get you the room with room_number == target_room_number. Use std::find again :)

15. Instead of checking for stream failure (in Dungeon::select_room_to_move) using fail, use std::cin's operator bool: if (!std::cin) /fail/;`.

16. if (game_over == true) can be simplified to if (game_over).

17. The introduction text is not up-to-date. There are three pits and super bats, not two. Might want to use the actual variables instead of hardcoding text.

18. It's my understanding that neighbors cannot ever be nullptr, so the extra if statement for it is unnecessary (wumpus.cpp:226). Might want to consider using an assert instead.

19. It seems like you are using player_room a lot. You might want to consider using a member variable for that so that you avoid to iterate over (potentially) the whole room container.

20. Dungeon::shoot_arrow has a bug. If I shoot my last arrow into a non-existent room, then I can illegally continue ;)

21. Dungeon::move_player has a bug, it moves the player into the wrong room. That's because rooms is not sorted, so rooms[target_room_number] doesn't get you the room with room_number == target_room_number. Use std::find again :)

15. Instead of checking for stream failure (in Dungeon::select_room_to_move) using fail, use std::cin's operator bool: if (!std::cin) /fail/;`.

16. (I'm not sure about this point). Isn't distribution in get_random not supposed to be static too?

17. if (game_over == true) can be simplified to if (game_over).

18. The introduction text is not up-to-date. There are three pits and super bats, not two. Might want to use the actual variables instead of hardcoding text.

19. It's my understanding that neighbors cannot ever be nullptr, so the extra if statement for it is unnecessary (wumpus.cpp:226). Might want to consider using an assert instead.

20. It seems like you are using player_room a lot. You might want to consider using a member variable for that so that you avoid to iterate over (potentially) the whole room container.

21. Dungeon::shoot_arrow has a bug. If I want to shoot an arrow into 3-5-9, then I only lose one arrow. I don't think that this intended. Also, if I shoot my last arrow into a non-existent room, then I can illegally continue ;).

22. Dungeon::move_player has a bug, it moves the player into the wrong room. That's because rooms is not sorted, so rooms[target_room_number] doesn't get you the room with room_number == target_room_number. Use std::find again :)

15. Instead of checking for stream failure (in Dungeon::select_room_to_move) using fail, use std::cin's operator bool: if (!std::cin) /fail/;`.

16. if (game_over == true) can be simplified to if (game_over).

17. The introduction text is not up-to-date. There are three pits and super bats, not two. Might want to use the actual variables instead of hardcoding text.

18. It's my understanding that neighbors cannot ever be nullptr, so the extra if statement for it is unnecessary (wumpus.cpp:226). Might want to consider using an assert instead.

19. It seems like you are using player_room a lot. You might want to consider using a member variable for that so that you avoid to iterate over (potentially) the whole room container.

20. Dungeon::shoot_arrow has a bug. If I want to shoot an arrow into 3-5-9, then I only lose one arrow. I don't think that this intended. Also, if I shoot my last arrow into a non-existent room, then I can illegally continue ;).

21. Dungeon::move_player has a bug, it moves the player into the wrong room. That's because rooms is not sorted, so rooms[target_room_number] doesn't get you the room with room_number == target_room_number. Use std::find again :)

15. Instead of checking for stream failure (in Dungeon::select_room_to_move) using fail, use std::cin's operator bool: if (!std::cin) /fail/;`.

16. (I'm not sure about this point). Isn't distribution in get_random not supposed to be static too?

17. if (game_over == true) can be simplified to if (game_over).

18. The introduction text is not up-to-date. There are three pits and super bats, not two. Might want to use the actual variables instead of hardcoding text.

19. It's my understanding that neighbors cannot ever be nullptr, so the extra if statement for it is unnecessary (wumpus.cpp:226). Might want to consider using an assert instead.

20. Instead of having a has_wumpus, has_pit, has_whatever, ..., it's better to use one enum and has_pit:

    enum class RoomInhabitant { Nobody, Player, Wumpus, Bat }; 

Then you can also use a switch instead of an if cascade if you'd like. This has also an additional advantage is that you cannot break invariants easily: In your posted code, you can have a room with has_pit == has_bat == true, which is not supposed to be possible. With an enum you do not have this problem.

21. It seems like you are using player_room a lot. You might want to consider using a member variable for that so that you avoid to iterate over (potentially) the whole room container.

22. Dungeon::shoot_arrow has a bug. If I want to shoot an arrow into 3-5-9, then I only lose one arrow. I don't think that this intended. Also, if I shoot my last arrow into a non-existent room, then I can illegally continue ;).

23. Dungeon::move_player has a bug, it moves the player into the wrong room. That's because rooms is not sorted, so rooms[target_room_number] doesn't get you the room with room_number == target_room_number. Use std::find again :)

15. Instead of checking for stream failure (in Dungeon::select_room_to_move) using fail, use std::cin's operator bool: if (!std::cin) /fail/;`.

16. (I'm not sure about this point). Isn't distribution in get_random not supposed to be static too?

17. if (game_over == true) can be simplified to if (game_over).

18. The introduction text is not up-to-date. There are three pits and super bats, not two. Might want to use the actual variables instead of hardcoding text.

19. It's my understanding that neighbors cannot ever be nullptr, so the extra if statement for it is unnecessary (wumpus.cpp:226). Might want to consider using an assert instead.

20. It seems like you are using player_room a lot. You might want to consider using a member variable for that so that you avoid to iterate over (potentially) the whole room container.

21. Dungeon::shoot_arrow has a bug. If I want to shoot an arrow into 3-5-9, then I only lose one arrow. I don't think that this intended. Also, if I shoot my last arrow into a non-existent room, then I can illegally continue ;).

22. Dungeon::move_player has a bug, it moves the player into the wrong room. That's because rooms is not sorted, so rooms[target_room_number] doesn't get you the room with room_number == target_room_number. Use std::find again :)