Skip to content
This repository was archived by the owner on Jan 3, 2023. It is now read-only.

Add new exercise: "Choose Your Own Adventure" game#27

Merged
40thieves merged 10 commits intomainfrom
add-more-exerises
Mar 17, 2021
Merged

Add new exercise: "Choose Your Own Adventure" game#27
40thieves merged 10 commits intomainfrom
add-more-exerises

Conversation

@40thieves
Copy link
Copy Markdown
Contributor

Description

Adds a new exercise to build a simple "Choose Your Own Adventure" game.

Solution / Solution (with stretch goal). If this is approved, I'll add it to the solutions repo.

Open questions:

  • Is the concept of room objects too complex?
    • The water bottle exercise uses a number value, so using an object (with methods) for the state is a step up
  • Is there another bit of state that we could track? I thought about adding some item picking up/using functionality, but thought it was getting overly complex for a single exercise

Who needs to know about this?

@ChrisOwen101
@illicitonion

@40thieves
Copy link
Copy Markdown
Contributor Author

Based on discussion with @illicitonion and @ChrisOwen101, I've tweaked the exercise so that it now prompts via the command line for input. This makes the game a bit more fun & realistic, but I couldn't figure out a nice way of running the "tests". I think it's probably fine, given that trainees can compare their input against the ascii art I put in the comments.

@ChrisOwen101 said that he would be able to run it as a quick test with some trainees. I've put the (unsolved) exercise in https://repl.it/@40thieves/CYF-Choose-Your-Own-Adventure-Game#index.js, so feel free to use that.

Copy link
Copy Markdown
Member

@illicitonion illicitonion left a comment

Choose a reason for hiding this comment

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

This looks great! I tried it out, and filed CodeYourFuture/JavaScript-Core-2-Coursework-Week1#52 with a few potential tweaks.

Two other things that occurred to me while trying it out:

  1. To really drive home the object-y-ness of things, it may be nice to provide an object of rooms, rather than a bunch of global variables? So that start could be implemented as something like this.currentRoom = rooms[roomName] (plus null check), rather than needing to hard-code each accepted room name and value in a big switch statement?
  2. I don't think it's obvious how we teach methods that methods are object properties like any other... (My move implementation did this.currentRoom[direction](), again avoiding a big switch statement)... I'm not sure how much this matters, and maybe the answer for this exercise is just for the trainees to use a switch statement... But it'd be kind of nice to start seeding the idea of "write the most general code you can to solve the problem"... Maybe the answer here is just to show both in the "Solutions" repo, and have a commentary about why one is more general/simple than the other in a comment in it...


let hallRoom = {
name: "Hall",
north: function () {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I assume these are functions because of the forward references? Because the reason they are functions is a bit obscure, I wonder if identifying them by name (along with the suggestion to have rooms be {hall:{...}, class:{...}, library:{...}} might make more sense?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I assume these are functions because of the forward references?

Yes mostly for that reason. But I ended up thinking that it might help to practice objects methods too.

This re-enforces the idea of nested objects and helps to give a cleaner solution with object square bracket notation
@40thieves
Copy link
Copy Markdown
Contributor Author

it may be nice to provide an object of rooms, rather than a bunch of global variables

That's really great feedback @illicitonion! I've gone ahead and pushed b7b2d5f with that change. I've also updated the solution repl.

I'm not sure how much this matters, and maybe the answer for this exercise is just for the trainees to use a switch statement... But it'd be kind of nice to start seeding the idea of "write the most general code you can to solve the problem"... Maybe the answer here is just to show both in the "Solutions" repo, and have a commentary about why one is more general/simple than the other in a comment in it...

I really like this idea, I'm a big fan of allowing for a naive solution while still having space to push more advanced students to more complex/abstract solutions 👍

I think we could perhaps have a solution.js and a solution-advanced.js (with comments) in the solutions repo.

The comments and prompts all use lower-case, and I feel like there's
enough for the trainees to be thinking about without having to think
about case-normalisation...
When I didn't guard setting `this.currentRoom` in `move`, the error
message told me to make sure my `start` method was correct. While
"Sometimes the error message is misleading" is a good thing for our
trainees to learn, I'm not sure this is the place, as there's already a
lot of new things to grasp in this exercise.
Comment thread mandatory/9-choose-your-own-adventure.js Outdated
@40thieves
Copy link
Copy Markdown
Contributor Author

40thieves commented Mar 13, 2021

Thanks for the reviews 🙂 Sounds like folks are happy with this so I'll prepare a solution in https://github.com/CodeYourFuture/JavaScript-Core-2-Coursework-Week1-Solution

@40thieves
Copy link
Copy Markdown
Contributor Author

@40thieves
Copy link
Copy Markdown
Contributor Author

I'm going to go ahead and get this merged now. The solution PR is still being reviewed, but I don't think it will effect the exercise.

@40thieves 40thieves merged commit f0ccb3f into main Mar 17, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants