Advertisement
Guest User

Untitled

a guest
Oct 19th, 2019
95
0
Never
Not a member of Pastebin yet? Sign Up, it unlocks many cool features!
text 4.52 KB | None | 0 0
  1. Overall:
  2. Source folders, and thereby packages are inconstantly capitalized, and most go against common Java conventions.
  3. A lot of classes seem to really just be there to contain information and little to no behaviour, with the behaviour instead reciding in single, massive classes.
  4.  
  5. Acquaintance
  6. This entire package seems to be purely interfaces. Having all interfaces in a single package seem troublesome at best, and at worst lacks seperation of concerns.
  7.  
  8. Some interfaces cover a large variety of subjects. IIMoveable, for instance, seem to just define generic items as well as specifically cabinets.
  9.  
  10. IGame is just fucking empty.
  11.  
  12. GlueCode
  13. I like this, a single class that just starts everything instead of the program starting directly in either the logic or GUI layer. Good job :>
  14.  
  15. Logic
  16. All line numbers refer to lines in the Game.java class, since it seems to be the only class that really does anything, aside from the player drowning.
  17.  
  18. Line 21 - String refers to secondWord of a command line interface. This couples the logic layer closely to a command line interface.
  19.  
  20. Line 22 - Room currentRoom indicates that there can really only be one active room in the game, which may not always be the case if the game was to support multiplayer. Additionally, the next line adds a Player object, which could easily have contained currentRoom instead. Which way to go depends on a varity of factors in the systems design, of course.
  21.  
  22. Line 24 - Inventory should not be managed directly by the core logic class.
  23.  
  24. Line 25 - dmgText is a bit odd to store in core logic class, however it may have been too impractical to implement a better solution, so I'll let this slide.
  25.  
  26. Line 26 - I see little reason to store the save game data in the logic class, instead of simply disposing of it once it has been created and saved permanetly, or loaded from permanemt storage.
  27.  
  28. Line 29 - See note on line 25.
  29. Further instances of dublicate issues will be ignored because I'm fucking lazy.
  30.  
  31. Line 32 -> 38 - Handling rooms directly in core logic class may become bit of a mess and breaks seperation of concerns.
  32.  
  33. Line 43 - Game () is declared to be able to throw an IOException but it only calls createRooms (), which throws no exceptions, meaning an IOException will never be thrown.
  34.  
  35. Line 51 -> 56 - A class behind the logic facade refers to the logic facade in order to get description texts? Layered architecture specifically involves never calling anything "above" the current class outside of invoking specific callback methods.
  36.  
  37. Line 138 - loseCondition is a very vauge name compared to the simple check it makes. I imagine it was supposed be able to check multiple conditions in a single method, but none other were implemented?
  38.  
  39. Line 146 - Method name implies it will directly print text to console, instead of returning a piece of text. Same with line 153
  40.  
  41. Line 161 - Direct use of console in logic layer. This is punisable by death. (We did it too though)
  42.  
  43. Line 168 - While I personally don't mind as much, some consider it bad practice to have a return statement in the middle of a function. Additionally, the "else" beneath will never be hit if the return is hit.
  44.  
  45. Line 194 -> 206 - This could have been done with a switch-case, and done even better with a concrete list of item instances instead, which.. is done just beneath? what.
  46. Your fucking inventory system makes no sense. Some items are in Game.java and some are in a specific player inventory? Is it like a list of static and dynamic items?
  47.  
  48. I'm growing tired of going through line by line, so henceforth it's just gonna be major stuff I notice.
  49.  
  50. Line 450 - This entire method is way too long and contains behaviour for individual items, as well as returns all over the place.
  51.  
  52. Line 527 - Management of monsters is done by the core logic class instead of a dedicated monster manager.
  53.  
  54. Line 592 - A quiz, while a fun idea, should also be contained by a dedicated class.
  55.  
  56. Line 720 - This method is way too long as well, and, like the previous of same issue, seemingly contains behaviour for individual instances of classes.
  57.  
  58. Line 886 - This method is way too far away from related methods, and the functionality of it doesn't really seem neccesary to seperate from the createRooms method.
  59.  
  60. Persistens
  61. This package is in a seperate langauge from the rest of the packages. Please execute whomever named it.
  62.  
  63. Most of the save functionality is inconsistant in quality with the rest of the project, indicating a use of advanced outside resources beyond your level. (We did that too as well, so no judgement lol.)
Advertisement
Add Comment
Please, Sign In to add comment
Advertisement