-
Notifications
You must be signed in to change notification settings - Fork 6
Igor w3 databases #20
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?
Conversation
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.
LGTM, only one small point for rework: I didn't see the answer for SQL injection question 1. Please let me know after you fill in the answer.
| function getPopulation(Country, name, code, cb) { | ||
| conn.query( | ||
| "SELECT Population FROM ?? WHERE Name = ? AND code = ?", | ||
| [Country, name, code], | ||
| function (err, result) { | ||
| if (err) return cb(err); | ||
| if (result.length === 0) return cb(new Error("Not found")); | ||
| cb(null, result[0].Population); | ||
| } | ||
| ); | ||
| } |
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.
Needs rework: I didn't see the answer to question 1 in exercise 3?
| // Deduct from account 101 | ||
| await conn.execute(`UPDATE account SET balance = balance - 1000 WHERE account_number = 101`); | ||
| await conn.execute( | ||
| `INSERT INTO account_changes (account_number, amount, remark) VALUES (?, ?, ?)`, | ||
| [101, -1000, "Transfer to 102"] | ||
| ); |
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.
Suggestion: no need to change your implementation for transactions, but it is better to check whether the accounts exist and whether the balance is enough before doing actual transactions.
| * It seems an errand episode has gotten into our data. | ||
| * This is episode 14 in season 31. Please remove it and verify that it has been removed! | ||
| */ | ||
| const bobRossCollection = client.db("databaseWeek3").collection("bob_ross_episodes"); |
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.
Suggestion: you may notice this line appears multiple times in this file. In this case, you can extract it into another function. You can also have two string constants for "databaseWeek3" and "bob_ross_episodes", so you can reuse those strings and avoid typos in future. And in future if you want to change database name, you don't need to change them one by one.
No description provided.