-
Notifications
You must be signed in to change notification settings - Fork 3
Oleksandr Starshynov w3 databases #19
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?
Oleksandr Starshynov w3 databases #19
Conversation
RHSebregts
left a comment
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.
Great work on this week's assignment. Overall I don't think you need to do a lot of refactoring/rewriting, just some small things to update/check.
I made a comment about whether or not AI was used to do (parts of) work. I want to reemphasise that using AI is fine, and a good skill to learn nowadays, but always keep in mind that the goal is for you to learn how these concepts work and apply them. The LLM's already "knows" how these things work (or make their best predictions about it, at the very least), and I want you to know as well so people decide to hire you instead of the AI overlords.
If I'm wrong here, please let me know! The output is good, so that's not what I'm worried about!
Have a nice weekend!
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.
This is all correct! The layout and language are triggering my AI-spider sense a bit. If you figured all of this out on your own, I'm completely ok with it if you asked ChappyG to format it. For me the most important thing is that you know what and why things are happening.
| const transactionFrom = { | ||
| account_number: fromAccount, | ||
| amount: -amount, | ||
| changed_date: date, | ||
| remark: `Transfer to account #${toAccount}`, | ||
| }; | ||
|
|
||
| const transactionTo = { | ||
| account_number: toAccount, | ||
| amount: amount, | ||
| changed_date: date, | ||
| remark: `Transfer from account #${fromAccount}`, | ||
| }; |
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.
I really like these objects! They are clear and well structured!
| database: "w3_assignment", | ||
| }); | ||
|
|
||
| await connection.beginTransaction(); |
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.
One could argue that we could move this into the try so that we can make sure to catch any errors with the starting of the transaction. But this is fine for now I think.
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.
I will try, but i think that to add tyr block here i need to add function for the hole transaction. I will try with try. :)
| await connection.query(` | ||
| CREATE TABLE IF NOT EXISTS account ( | ||
| account_number INT PRIMARY KEY, | ||
| balance DECIMAL(12, 2) NOT NULL |
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.
A 100 billion is a nice big number for this, well done!
| } | ||
|
|
||
| async function findEpisodesExercises(client) { | ||
| const db = client.db("databaseWeek3"); |
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.
Is there a way to make this a bit DRY-er and not have to select a db/collection for every query?
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.
I have read about this and I am not sure. We can create Reusable Collection References here, but I think it is part of week4 in reading. If you expect here something another to be done by me, please tell me more.
| const db = client.db("databaseWeek3"); | ||
|
|
||
| // 1. Title of episode 2 in season 2 | ||
| const ep1 = await db.collection("bob_ross_episodes").findOne({ episode: "S02E02" }); |
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.
Naming episode 2 of season 2 ep1 is breaking my brain. I get your intention here, but let's try and find a name that is doing what you want, without confusing me so much.
| // 3. All episodes with CLIFF | ||
| const cliffEpisodes = await db.collection("bob_ross_episodes") | ||
| .find({ elements: "CLIFF" }) | ||
| .project({ title: 1, _id: 0 }) |
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.
Using project here is a great idea. Can you give me a short description of what this does and how it works?
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.
As I understend project iin Mongo is the same as we used SELECT in SQL. So, if we need to return not everything but only expected filds, we can "cut" responce.
|
|
||
| // 1. Rename title of S30E13 | ||
| const updateResult1 = await db.collection("bob_ross_episodes").updateOne( | ||
| { episode: "S30E13", title: "BLUE RIDGE FALLERS" }, |
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.
Do we need both identifiers here to find this entry?
| { elements: "BUSHES" }, | ||
| { | ||
| $addToSet: { elements: "BUSH" }, | ||
| $pull: { elements: "BUSHES" } | ||
| } |
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.
Nice!
| episode: "S31E14" | ||
| }); | ||
| console.log( | ||
| `Ran a command to delete episode and it deleted ${deleteResult.deletedCount} 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.
So here we have a check that we have indeed deleted one, but does this verify that we have successfully removed the one we wanted?
No description provided.