Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Trial #4

Open
wants to merge 8 commits into
base: main
Choose a base branch
from
Open

Trial #4

wants to merge 8 commits into from

Conversation

brittktrinh
Copy link
Owner

@brittktrinh brittktrinh commented Jul 28, 2023

  • Successfully accessed the object keys to obtain "Name" and "Percentage" values
  • Randomly generates answers + randomly generates cities using Math functions
  • Formatted output to look like a multiple choice question

Next steps:

  • Randomizing correct answer choice
  • Taking user input to compare to correct answer
  • Create a way to stop or continue the program (currently to exit we use "CTRL + C")

Copy link
Collaborator

@Eric-Arellano Eric-Arellano left a comment

Choose a reason for hiding this comment

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

Good work! Once you fix the lint issues I mentioned, this looks good.

// input: process.stdin,
// output: process.stdout,
// });
const parking_csv = axios
Copy link
Collaborator

Choose a reason for hiding this comment

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

You have some lint failures because the convention in JavaScript is to use camelCase. Rather than parking_csv, it would be parkingCsv.

But also, ESLint is telling you that you never use the parking_csv variable in the first place, so you can remove it.

// "https://raw.githubusercontent.com/ParkingReformNetwork/mandates-map/main/map/trimmed_map_data.csv";
// const CITIES_JSON =
// "https://raw.githubusercontent.com/ParkingReformNetwork/parking-lot-map/main/data/score-cards.json";
const rl = readline.createInterface({
Copy link
Collaborator

Choose a reason for hiding this comment

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

rl is not a very informative variable. A better name would be something like userInputInterface or readlineInterface.

const citiesArray = Object.keys(response.data);

// Select a city to guess the answer to
let elem = Math.floor(
Copy link
Collaborator

Choose a reason for hiding this comment

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

As ESLint is suggesting, use const. You should always default to const. let is only when you need to reassign a variable like this:

let myVar = 1;
myVar = 2;

@Eric-Arellano
Copy link
Collaborator

By the way, PR titles should be more descriptive than Trial. Something like "Set up basic user interface"

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants