-
Notifications
You must be signed in to change notification settings - Fork 3
Ilias_Khugaev-w3-Databases #21
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?
Ilias_Khugaev-w3-Databases #21
Conversation
yunchen4
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.
Hi Ilias,
There are some points you need to rework. I have made them explicit. Otherwise it looks good.
Please let me know on Slack when you finish the rework, and don't hesitate to contact me if you have any questions!
Week3/assignment/answers.md
Outdated
| 1. **What columns violate 1NF?** | ||
| Columns: `member_address`, `dinner_date`, `food_code`, `food_description` violate 1NF, because each column should be single-valued. |
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: It is correct that food_code, food_description should be single-valued. But why other two fields? Looks to me that these two fields is already single-valued (only has one value per row). Could you please check again?
dinner_date indeed has some issue, but not the single-valued issue.
Week3/assignment/transaction.js
Outdated
| } catch (err) { | ||
| console.log(err); |
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: the reason why we want to use transactions is mainly for error cases - we want to revert everything back if something is wrong. Here you only log errors, but there is no step to revert things back.
Week3/homework/mongodb/index.js
Outdated
| elements: ["CIRRUS", "CLOUDS", "CONIFER", "DECIDIOUS", "GRASS", "MOUNTAIN", "MOUNTAINS", "RIVER", "SNOWY_MOUNTAIN", "TREE", "TREES"] | ||
| }; | ||
|
|
||
| const result = await client.db('databaseWeek3').collection('bob_ross_episodes').insertOne({doc}); |
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: I noticed the string 'databaseWeek3' occurs several times in this file. In this case you can use a costant. There are several advantages to extract a string to a constant if it appears repeatingly:
- Avoid typos.
- In future if you want to change database name, you don't need to change it everywhere.
- Usually the database name is part of the config file. When you import a config file it is usually a variable.
Same thing applies to everything that might be repeated.
Week3/homework/mongodb/index.js
Outdated
| const doc = { | ||
| episode: "S09E13", | ||
| title: "MOUNTAIN HIDE-AWAY", | ||
| elements: ["CIRRUS", "CLOUDS", "CONIFER", "DECIDIOUS", "GRASS", "MOUNTAIN", "MOUNTAINS", "RIVER", "SNOWY_MOUNTAIN", "TREE", "TREES"] | ||
| }; |
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: since this search criteria is only used once, you don't need to extract it to be a separate variable. You can just express it when you do the mongodb query.
| const allCLIFFEpisodes = []; | ||
| for await (const doc of result2) { | ||
| allCLIFFEpisodes.push(doc.title); | ||
| } |
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 can use Array.prototype.map() (doc) to get a new array with only titles. Then you don't need to use for loop.
Week3/homework/mongodb/index.js
Outdated
| const result2 = await client.db('databaseWeek3').collection('bob_ross_episodes').updateMany({elements: "BUSHES"}, { $set: {elements: "BUSH"}}); | ||
|
|
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: here you use { $set: {elements: "BUSH"}}. This will update the array field elements to be BUSH (so after updating elements is a string, not an array), not the element inside the field elements.
Week3/assignment/answers.md
Outdated
| 3. **Name all the tables and columns that would make a 3NF compliant solution.** | ||
| - `member_id` and `member_name` | ||
| - `venue_code` and `venue_description` No newline at end of file |
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.
You have answered the question 3 above. Although it is not 3NF, what are the possible relationship tables?
| const result2 = await client | ||
| .db(dataBaseName) | ||
| .collection(collectionName) | ||
| .updateMany( | ||
| { elements: "BUSHES" }, [{$set: {elements: {$map: {input: "$elements", as: "el", in: {$cond: [{ $eq: ["$$el", "BUSHES"] }, "BUSH", "$$el"]}}}}}] | ||
| ); |
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: Your answer is correct. However, you can just use $ to refer to the element:
.updateMany(
{
elements: "BUSHES",
},
{ $set: { "elements.$": "BUSH" } }
);
| 1. **Which columns break 1NF?** | ||
| - Each cell must have one single value (atomic). | ||
| - No lists or repeated values in one cell. | ||
| <br>**These columns break 1NF:** | ||
| <br>`food_code → example: C1, C2` | ||
| <br>`food_description → example: Curry, Cake` |
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.
Your answer is correct. dinner_date also has a issue: its format is not uniform. Sometimes it is a ISO string but sometimes not. We also need to avoid this kind of situation.
For date/time, MySQL has types for it. We should use that instead of use varchar.
| - Member_Dinner | ||
| `member_id Foreign Key → Members.member_id` | ||
| `dinner_id Foreign Key → Dinners.dinner_id` |
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.
Dinner and food also have many-to-many relationships.
No description provided.