Feedback #1

github-classroom · 2021-02-14T23:05:01Z

👋! GitHub Classroom created this pull request as a place for your teacher to leave feedback on your work. It will update automatically. Don’t close or merge this pull request, unless you’re instructed to do so by your teacher.

In this pull request, your teacher can leave comments and feedback on your code. Click the Subscribe button to be notified if that happens.

Click the Files changed or Commits tab to see all of the changes pushed to master since the assignment started. Your teacher can see this too.

Notes for teachers

Use this PR to leave feedback. Here are some tips:

Click the Files changed tab to see all of the changes pushed to master since the assignment started. To leave comments on specific lines of code, put your cursor over a line of code and click the blue + (plus sign). To learn more about comments, read “Commenting on a pull request”.
Click the Commits tab to see the commits pushed to master. Click a commit to see specific changes.
If you turned on autograding, then click the Checks tab to see the results.
This page is an overview. It shows commits, line comments, and general comments. You can leave a general comment below.

For more information about this pull request, read “Leaving assignment feedback in GitHub”.

Subscribed: @swayamr2

…store information directly from JSON

…enture, containing the array of rooms with all variables, the starting room, and the ending room

… inputs, etc.

AkaashKolachina · 2021-02-18T04:00:25Z

src/main/java/student/Main.java

+            //Store JSON data into an AdventureDesign Object
+            currentDesign = mapper.readValue(file, AdventureDesign.class);
+        } catch (Exception e) {
+            System.out.println("Please try again!");


It's a little odd that you tell the user to try again, but you don't give them the opportunity to try again.

AkaashKolachina · 2021-02-18T04:00:44Z

src/main/java/student/Main.java

+        for (Room room : rooms) {
+            if (room.getName().equalsIgnoreCase(currentDesign.getStartingRoom())) {
+                currentRoom = room;
+                //store beginning room name into variable startingRoom


Be consistent with comment casing

AkaashKolachina · 2021-02-18T04:02:52Z

src/main/java/student/Main.java

+ * "drop", "examine", and more. Accounts for spelling and spacing errors.
+ */
+public class Main {
+    private static AdventureDesign currentDesign;


I'm not a fan of the name currentDesign because that implies the design could change mid-game which it won't.

AkaashKolachina · 2021-02-18T04:04:40Z

src/main/java/student/Main.java

+                //store beginning room name into variable endingRoom
+                endingRoom = room.getName();
+            }
+        }


A line of whitespace would be helpful here

AkaashKolachina · 2021-02-18T04:07:04Z

src/main/java/student/adventure/UserMoves.java

+import java.util.Collections;
+
+/**
+ * I created a static class UserMoves, containing all the moves that will be used in the main method.


You just need to say what a class does in its docs, you don't need to explain why you created it

AkaashKolachina · 2021-02-18T04:07:27Z

src/main/java/student/adventure/UserMoves.java

+     * This method takes the entire details of a certain room, including description, items and directions.
+     * @param currentRoom - The room that is being traversed
+     */
+    public static void roomDetails(Room currentRoom) {


Method names should start with a verb

AkaashKolachina · 2021-02-18T04:10:27Z

src/main/java/student/adventure/UserMoves.java

+ */
+public class UserMoves {
+    /**
+     * This method takes the entire details of a certain room, including description, items and directions.


This description doesn't really tell me what the method does

AkaashKolachina · 2021-02-18T04:12:52Z

src/main/java/student/adventure/UserMoves.java

+        System.out.println("Here are the items in this room: " + Arrays.toString(currentRoom.getItems()));
+        ArrayList<String> allPossibleDirections = new ArrayList<>();
+        DirectionsOnMap[] directions = currentRoom.getDirections();
+        for(DirectionsOnMap directionName: directions) {


Calling this variable name directionName is a bit of a misnomer because it is a direction object, not a direction name. This also results in the line directionName.getDirectionName() which is needlessly confusing.

AkaashKolachina · 2021-02-18T04:13:29Z

src/main/java/student/adventure/UserMoves.java

+        System.out.println("Here are the items in this room: " + Arrays.toString(currentRoom.getItems()));
+        ArrayList<String> allPossibleDirections = new ArrayList<>();
+        DirectionsOnMap[] directions = currentRoom.getDirections();
+        for(DirectionsOnMap directionName: directions) {


There should be a space before the (

AkaashKolachina · 2021-02-18T04:14:17Z

src/main/java/student/adventure/UserMoves.java

+        ArrayList<String> allPossibleDirections = new ArrayList<>();
+        DirectionsOnMap[] directions = currentRoom.getDirections();
+        for(DirectionsOnMap directionName: directions) {
+            //Stores all of the possible direction names one can travel in using a String ArrayList


You don't need to say the data structure that is storing the data, it is obvious to the reader from the type specification

AkaashKolachina · 2021-02-18T04:17:11Z

src/main/java/student/adventure/UserMoves.java

+            System.out.println("Thank you for playing! Goodbye");
+            System.exit(0);
+        }
+    }


Have vertical whitespace between functions!

AkaashKolachina · 2021-02-18T04:18:11Z

src/main/java/student/adventure/UserMoves.java

+    }
+
+    /**
+     * This method decides whether a room is at the start or end of the adventure, and prints out statements


"prints out statements" is really vague and would not help a user understand what this method does

AkaashKolachina · 2021-02-18T04:49:02Z

src/main/java/student/adventure/UserMoves.java

+     * @param startingRoom - the starting room of the adventure
+     * @param endingRoom - the ending room of the adventure
+     */
+    public static void beginningOrEnd(Room currentRoom, String startingRoom, String endingRoom) {


I don't really understand the purpose of this function, since you only call it when you know the game started or ended, so why not just print the proper statement then?

AkaashKolachina · 2021-02-18T04:52:25Z

src/main/java/student/adventure/UserMoves.java

+            }
+        }
+        //Returns out the current state of the room if it cannot find a match
+        System.out.println("I cannot understand " + newDirection);


This is relatively minor but the proper message should be "I can't go "direction"!"

AkaashKolachina · 2021-02-18T04:53:06Z

src/main/java/student/adventure/UserMoves.java

+    }
+
+    /**
+     * This method takes an item, stores it into a list of String variables, and removes it from the overall


This javadoc is very operational

AkaashKolachina · 2021-02-18T04:53:57Z

src/main/java/student/adventure/UserMoves.java

+     * @param itemList - the user list of the items
+     */
+    public static void takeItem(Room currentRoom, String pickedItem, ArrayList<String> itemList) {
+        if(pickedItem == null) {


There should be a space before (

AkaashKolachina · 2021-02-18T04:59:15Z

src/main/java/student/adventure/UserMoves.java

+        for(String item : currentRoom.getItems()) {
+            //Loops through the current item state to find a match
+            if (item.equalsIgnoreCase(pickedItem.trim())) {
+                //Adds the item to the user list of items


I love that you are adding more in-line comments, but don't be redundant with it. You don't need a comment explaining each line. It's obvious you're adding and removing items from your lists

AkaashKolachina · 2021-02-18T05:00:50Z

src/main/java/student/adventure/UserMoves.java

+        if(pickedItem == null) {
+            System.out.println("This is a null input");
+        }
+        ArrayList<String> fullItemList = new ArrayList<>();


I don't understand why you have this variable since you already have a list of all the items in the room

AkaashKolachina · 2021-02-18T05:04:05Z

src/main/java/student/adventure/UserMoves.java

+                //Adds the item to the user list of items
+                itemList.add(pickedItem);
+                //removes the item room
+                fullItemList.remove(pickedItem);


Why do you keep switching between Arrays and ArrayLists, it seems really random and is making your code hard to follow.

AkaashKolachina · 2021-02-18T05:05:33Z

src/main/resources/BreakingBad.json

@@ -0,0 +1,162 @@
+{


Haha this is a really cool game idea!

AkaashKolachina · 2021-02-18T05:12:03Z

src/main/java/student/Main.java

+        while (!(currentRoom.getName().equals(endingRoom))) {
+            String input = scanner.nextLine();
+            input = input.toLowerCase();
+            if (input.contains("go")) {


The issue with checking like this is that it's not enough for go to be in the input, it needs to be in the front. Also there needs to be two words in a go command: go and the direction. The way you implemented the go check, it would pass even if there's only one word (like golf) which is incorrect behavior.

AkaashKolachina · 2021-02-18T05:23:28Z

src/main/java/student/Main.java

+ * Main class that uses a scanner input to call methods, and is used as a UI. Commands include "go", "take",
+ * "drop", "examine", and more. Accounts for spelling and spacing errors.
+ */
+public class Main {


Your main has way too much in it right now. All of your game logic should be in its own class. At the moment its impossible to test your game engine, which is a big issue as we always want to make our code as easy to test as possible.

AkaashKolachina · 2021-02-18T05:25:03Z

src/main/java/student/Main.java

+            currentDesign = mapper.readValue(file, AdventureDesign.class);
+        } catch (Exception e) {
+            System.out.println("Please try again!");
+            System.exit(0);


I'd strongly advise you to stay away from System.exit(). This function kills your entire jvm, which makes your program very inflexible and also a little dangerous.

AkaashKolachina · 2021-02-18T05:26:03Z

src/main/java/student/adventure/DirectionsOnMap.java

+ * This class works much like a linked list, where the Direction acts as a key, and the next room is viewed
+ * as a value
+ */
+public class DirectionsOnMap {


Why not just call this class Directions? DirectionsOnMap feels a bit redundant.

AkaashKolachina · 2021-02-18T05:27:01Z

src/main/java/student/adventure/DirectionsOnMap.java

+    //the direction one is going in
+    private String directionName;
+    //the next room one will go in following the currentDirection
+    private String room;


I'd advise you to call this member variable roomName since you have an object type called Room.

AkaashKolachina · 2021-02-18T05:31:07Z

src/test/java/student/adventure/AdventureTest.java

+        Room currentRoom = rooms[0]; //Walter White Residence
+        UserMoves.takeItem(currentRoom, "money", userTest);
+        UserMoves.takeItem(currentRoom, "hat", userTest);
+        assertEquals(2, userTest.size());


This doesn't convince me take works properly. I know 2 items are added to the player list, but I don't know if they are the correct items.

You also should verify that the item has been removed from the room

AkaashKolachina · 2021-02-18T05:33:35Z

src/main/java/student/adventure/UserMoves.java

+     * @param pickedItem - the item that is being taken
+     * @param itemList - the user list of the items
+     */
+    public static void takeItem(Room currentRoom, String pickedItem, ArrayList<String> itemList) {


Don't modify your parameters in your functions

…ine class to fit with the API requirements

github-classroom bot and others added 9 commits February 14, 2021 23:05

Setting up GitHub Classroom Feedback

0bade8a

Created Breaking Bad json file

55534d2

Created both the Room and DirectionsOnMap classes, which are used to …

947adfe

…store information directly from JSON

Completed AdventureDesign class, which is the basic layout of the Adv…

31ab2a4

…enture, containing the array of rooms with all variables, the starting room, and the ending room

Completed test suites, checking for irregular spelling, spacing, null…

9f95ac8

… inputs, etc.

Made final Javadoc and regular comments in my code

e80ae0f

Made final changes

2ad30ee

Made final changes

a03a955

Added "drop" tests in my suite

4e18322

AkaashKolachina reviewed Feb 18, 2021

View reviewed changes

src/main/resources/BreakingBad.json

@@ -0,0 +1,162 @@

{

Copy link

AkaashKolachina Feb 18, 2021

Choose a reason for hiding this comment

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

Haha this is a really cool game idea!

AkaashKolachina reviewed Feb 18, 2021

View reviewed changes

swayamr2 added 5 commits February 23, 2021 17:42

Added new methods for the web input, updated my Adventure Service

2a970ac

Completed web input method with comments, and reconfigured my gameEng…

2b562d1

…ine class to fit with the API requirements

Completed test suites

0e5cf44

Finished test suites and Scanner class for Console Interaction

53c7483

Finished test suites and Scanner class for Console Interaction

4da83b5

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Feedback #1

Feedback #1

github-classroom bot commented Feb 14, 2021 •

edited

Loading

AkaashKolachina Feb 18, 2021

AkaashKolachina Feb 18, 2021

AkaashKolachina Feb 18, 2021

AkaashKolachina Feb 18, 2021

AkaashKolachina Feb 18, 2021

AkaashKolachina Feb 18, 2021

AkaashKolachina Feb 18, 2021

AkaashKolachina Feb 18, 2021

AkaashKolachina Feb 18, 2021

AkaashKolachina Feb 18, 2021

AkaashKolachina Feb 18, 2021

AkaashKolachina Feb 18, 2021

AkaashKolachina Feb 18, 2021

AkaashKolachina Feb 18, 2021 •

edited

Loading

AkaashKolachina Feb 18, 2021

AkaashKolachina Feb 18, 2021

AkaashKolachina Feb 18, 2021

AkaashKolachina Feb 18, 2021

AkaashKolachina Feb 18, 2021

AkaashKolachina Feb 18, 2021

AkaashKolachina Feb 18, 2021

AkaashKolachina Feb 18, 2021

AkaashKolachina Feb 18, 2021

AkaashKolachina Feb 18, 2021

AkaashKolachina Feb 18, 2021

AkaashKolachina Feb 18, 2021

AkaashKolachina Feb 18, 2021

AkaashKolachina Feb 18, 2021

Feedback #1

Are you sure you want to change the base?

Feedback #1

Conversation

github-classroom bot commented Feb 14, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

AkaashKolachina Feb 18, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

github-classroom bot commented Feb 14, 2021 •

edited

Loading

AkaashKolachina Feb 18, 2021 •

edited

Loading