-
Notifications
You must be signed in to change notification settings - Fork 18
Pavel T #4
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
base: main
Are you sure you want to change the base?
Pavel T #4
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,3 +1,58 @@ | ||
| let result = 0; | ||
|
|
||
| export class Time { | ||
| // Your code here | ||
| } | ||
| #secondsFromMidnight; | ||
| constructor(hours, minutes, seconds) { | ||
| this.#secondsFromMidnight = hours * 3600 + minutes * 60 + seconds; | ||
| this.hours = hours; | ||
| this.minutes = minutes; | ||
| this.seconds = seconds; | ||
| } | ||
|
|
||
| getHours() { | ||
| return this.hours; | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Rather than maintaining a separate class property for the hours value, you can simply compute the value using integer arithmetic from the |
||
| } | ||
|
|
||
| getMinutes() { | ||
| return this.minutes; | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Same here |
||
| } | ||
|
|
||
| getSeconds() { | ||
| return this.seconds; | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. And here |
||
| } | ||
|
|
||
| #normilized() { | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Nitpick: spelling error -> normalized |
||
| if (this.#secondsFromMidnight < 0) { | ||
| this.#secondsFromMidnight = | ||
| 86400 - Math.abs(this.#secondsFromMidnight % 86400); | ||
| } | ||
| if (this.#secondsFromMidnight >= 86400) { | ||
| this.#secondsFromMidnight = this.#secondsFromMidnight % 86400; | ||
| } | ||
| this.hours = Math.floor(this.#secondsFromMidnight / 3600); | ||
| this.minutes = Math.floor((this.#secondsFromMidnight % 3600) / 60); | ||
| this.seconds = | ||
| this.#secondsFromMidnight - this.hours * 3600 - this.minutes * 60; | ||
| return this.#secondsFromMidnight; | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If you drop the |
||
| } | ||
|
|
||
| addSeconds(sec) { | ||
| this.#secondsFromMidnight = this.#secondsFromMidnight + sec; | ||
| return this.#normilized(); | ||
| } | ||
|
|
||
| addMinutes(min) { | ||
| this.#secondsFromMidnight = this.#secondsFromMidnight + min * 60; | ||
| return this.#normilized(); | ||
| } | ||
|
|
||
| addHours(hrs) { | ||
| this.#secondsFromMidnight = this.#secondsFromMidnight + hrs * 3600; | ||
| return this.#normilized(); | ||
| } | ||
|
|
||
| toString() { | ||
| return `${String(this.hours).padStart(2, "0")}:${String(this.minutes).padStart(2, "0")}:${String(this.seconds).padStart(2, "0")}`; | ||
| // return `${("0" + this.hours).slice(-2)}:${("0" + this.minutes).slice(-2)}:${("0" + this.seconds).slice(-2)} | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Remove commented code in pull request. No need for a reviewer to see what you tried but did not use. |
||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,4 +1,5 @@ | ||
| import { Time } from './Time.js'; | ||
|
|
||
| const time = new Time(13, 37, 0); | ||
| console.log(time.toString()); // Output: "13:37:00" | ||
| const time = new Time(23, 59, 50); | ||
| // console.log(time.toString()); // Output: "13:37:00" | ||
| console.log(time.addSeconds(20)) |
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,14 @@ | ||
| export const BASE_PROMPT = `You are a quiz master specializing in Ancient Egypt. | ||
| Generate exactly 1 unexpected quiz question about Ancient Egypt. | ||
|
|
||
| Return ONLY a valid JSON with no markdown, no explanation, no code fences. | ||
| Each element must have this exact shape: | ||
| { | ||
| "question": "string", | ||
| "answers": ["string", "string", "string", "string"], | ||
| "correct": 1 | ||
| } | ||
| "correct" is the 1-based index of the right answer (1–4). | ||
|
|
||
| Vary difficulty: questions 1-3 easy, 4-7 medium, 8-10 hard. | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The LLM does not remember its previous responses, unless you include that in the prompt (i.e. build up a context window in your prompt). So it will not know whether any previous questions were easy, medium or hard. If you used the tip from the assignment description: Ask the LLM to generate all 10 questions in one reply before the start of the quiz, then it would be able to create a variety of questions with mixed difficulty because it would be contained in a single response, with the added advantage that it would cost you one request instead of 10. Another issue with repeating the same prompt for each question is that there is no guarantee that the questions will be unique, i.e. no duplicates. |
||
| Cover a mix of subtopics: pharaohs, gods, monuments, daily life, writing, history.` | ||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -1 +1,71 @@ | ||||||||||||||||||
| // Write your code here. You may create as many files as you like. | ||||||||||||||||||
| import "dotenv/config"; | ||||||||||||||||||
| import { OpenAI } from "openai/client.js"; | ||||||||||||||||||
| import promptSync from "prompt-sync"; | ||||||||||||||||||
| import chalk from "chalk"; | ||||||||||||||||||
| import { BASE_PROMPT } from "./basePrompt.js"; | ||||||||||||||||||
| import {userAnswer, isValidAnswer} from "./utils.js" | ||||||||||||||||||
|
|
||||||||||||||||||
| const prompt = promptSync(); | ||||||||||||||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. A user who wants to break off the quiz should either be able to press Ctrl-C or you should provide a menu option (say,
Suggested change
|
||||||||||||||||||
|
|
||||||||||||||||||
| const openai = new OpenAI({ | ||||||||||||||||||
| baseURL: "https://models.github.ai/inference/", | ||||||||||||||||||
| apiKey: process.env.API_KEY, | ||||||||||||||||||
| }); | ||||||||||||||||||
|
|
||||||||||||||||||
| async function app() { | ||||||||||||||||||
| try { | ||||||||||||||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 👍 for handling errors |
||||||||||||||||||
| console.log("Welcome to the quiz ANCIENT EGYPT! Ready to begin?"); | ||||||||||||||||||
| prompt("Press enter to start the game"); | ||||||||||||||||||
|
|
||||||||||||||||||
| const score = { | ||||||||||||||||||
| totalQuestions: 10, | ||||||||||||||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This property is never mutated. It might as well have been a const TOTAL_QUESTIONS = 10; |
||||||||||||||||||
| currentQuestion: 1, | ||||||||||||||||||
| correctAnswers: 0, | ||||||||||||||||||
| wrongAnswers: 0, | ||||||||||||||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This property is updated but never reported. It serves no purpose. |
||||||||||||||||||
| }; | ||||||||||||||||||
|
|
||||||||||||||||||
| while (score.currentQuestion <= score.totalQuestions) { | ||||||||||||||||||
| console.log(`Loading question ${score.currentQuestion}...`); | ||||||||||||||||||
| const response = await openai.chat.completions.create({ | ||||||||||||||||||
| model: "openai/gpt-4o-mini", | ||||||||||||||||||
| response_format: { type: "json_object" }, | ||||||||||||||||||
| messages: [{ role: "user", content: BASE_PROMPT }], | ||||||||||||||||||
| }); | ||||||||||||||||||
|
|
||||||||||||||||||
| const responseContent = response.choices[0].message.content; | ||||||||||||||||||
| const result = JSON.parse(responseContent); | ||||||||||||||||||
|
|
||||||||||||||||||
| console.log(` | ||||||||||||||||||
| ${chalk.blue(result.question)} | ||||||||||||||||||
| ${result.answers.map((answer, i) => `${i + 1}. ${answer}`).join("\n")} | ||||||||||||||||||
| `); | ||||||||||||||||||
|
Comment on lines
+39
to
+41
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. To maintain correct indentation you could refactor this as:
Suggested change
|
||||||||||||||||||
|
|
||||||||||||||||||
| let answer = userAnswer(); | ||||||||||||||||||
|
|
||||||||||||||||||
| while (true) { | ||||||||||||||||||
| if (isValidAnswer(answer)) { | ||||||||||||||||||
| break; | ||||||||||||||||||
| } else { | ||||||||||||||||||
| console.error("Please enter an integer number between 1 and 4: "); | ||||||||||||||||||
| answer = userAnswer(); | ||||||||||||||||||
| } | ||||||||||||||||||
| } | ||||||||||||||||||
|
|
||||||||||||||||||
| if (result.correct === answer) { | ||||||||||||||||||
| console.log(chalk.green("Correct! Well done.")); | ||||||||||||||||||
| score.correctAnswers += 1; | ||||||||||||||||||
| } else { | ||||||||||||||||||
| console.log(chalk.red(`Wrong! The correct answer was: ${result.correct}`)); | ||||||||||||||||||
| score.wrongAnswers += 1; | ||||||||||||||||||
| } | ||||||||||||||||||
| score.currentQuestion += 1; | ||||||||||||||||||
| } | ||||||||||||||||||
|
|
||||||||||||||||||
| console.log( | ||||||||||||||||||
| chalk.yellow(`Quiz finished! Your final score is ${score.correctAnswers}/${score.totalQuestions}`), | ||||||||||||||||||
| ); | ||||||||||||||||||
| } catch (error) { | ||||||||||||||||||
| console.error(error.message); | ||||||||||||||||||
| } | ||||||||||||||||||
| } | ||||||||||||||||||
| app(); | ||||||||||||||||||
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,8 @@ | ||
| import promptSync from "prompt-sync"; | ||
|
|
||
| const prompt = promptSync(); | ||
|
|
||
| export const userAnswer = () => Number((prompt("Your answer (1-4): ") || "").trim()); | ||
|
|
||
| export const isValidAnswer = (userAnswer) => | ||
| userAnswer >= 1 && userAnswer <= 4 && Number.isInteger(userAnswer); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is not very clear which variable(s) represent the "single source of truth" of the time value. The class encapsulates the time as a number of seconds as well as an hours, minutes and seconds value. Moreover, the
#secondsFromMidnightproperty is made private, while thehours,minutesandsecondsproperties are public despite the fact that there are "getter" methods for these values.It would be clearer to have a single private property,
#secondsFromMidnight, to represent the time value and compute the hours, minutes, and seconds on the fly in the "getter" methods.