sandeeplayam / Jump-In-Board-Game

Sysc 3110 project
2 stars 0 forks source link

Milestone 1 feedback #1

Open RJBansal opened 4 years ago

RJBansal commented 4 years ago

Good job on UML, design document and game play.

Github needs to be organized better (files in respective folders and proper branch name)

Readme is not adequate. Should be better formatted and explain game play, instructions to play and run

Board should not be responsible for movement for foxes and rabbits → low cohesion currently

Using Strings for direction and words is smelly, use enums

Using instance of too often is a sign of bad design, delegate to get rid of that problem

Another interface should be used for all moving pieces, currently mushroom, fox and rabbit classes are doing nothing

Gameplay and parsing should be separated

Overall good architecture, however most your classes are doing nothing. Spread out the duties concentrated in the few classes. That will make your code much better

brougy commented 4 years ago

Can you tell us what exactly did we lose marks on? Your explanations are kinda vague. Thanks! @RJBansal

RJBansal commented 4 years ago

The marks lost are the result of a design that needs improvement which is 2 marks lost. Readme lost .5. I will be updating your mark to 7.5, after looking at it again. Hope that explains your question

davidou997 commented 4 years ago

Hey @RJBansal, I was wondering if you can clarify on certain comments in the milestone feedback.

"Readme is not adequate. Should be better formatted and explain game play, instructions to play and run"

The instructions on the project PDF never states that it is required to explain the gameplay, and instructions to play and run. It states that we must have the "the rest of the deliverables, the authors, the changes that were made since the previous deliverable, the known issues (known issues are graded less severely than undocumented ones!) and the roadmap ahead." Those requirements have been fulfilled already.

"Another interface should be used for all moving pieces, currently mushroom, fox and rabbit classes are doing nothing"

Our current design is supposed to be a reflection of how the game works in real life. The player makes the movement decision and picks up the pieces from the board to move them. The game pieces simply sit on the board, waiting for the player to make a decision and be picked up. I was wondering if you have any suggestions on what the mushroom, fox, and rabbit classes can do and any suggestion as to how we should change our game design.

Thank you.

RJBansal commented 4 years ago

Currently, your pieces can be represented as enums, why have classes for them? Each piece needs to be responsible for its own movement rules, that is not the responsible of the board.

For the readme, roadmap is missing. I will give you the .5 though. 8/10

tharsan18 commented 4 years ago

Hey @RJBansal we are trying to fix the issues you mentioned but we wanted to clarify what you said about having the pieces represented by enums (quoted below).

Another interface should be used for all moving pieces, currently mushroom, fox and rabbit classes are doing nothing

Currently, your pieces can be represented as enums, why have classes for them?

Do you mean having 1 enum class that implements the interface of moving pieces? Thanks!

brougy commented 4 years ago

He might've meant that since our Rabbit/Fox classes are not currently doing anything, they can just be represented as enums. If that's the case and you're implying that we make those R/F classes do something then how do we go about that? Make the rabbit ask the board if it's possible to hop?

The board has all the slots so we figured the board is the only class capable of seeing and setting all the movements.