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

fady-w3-database #23

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

Conversation

Fadysaadeddin
Copy link

No description provided.

@Fadysaadeddin Fadysaadeddin changed the title fady-w1-database fady-w3-database Jan 15, 2025
@dyaboykyl dyaboykyl self-assigned this Jan 16, 2025
Copy link

@dyaboykyl dyaboykyl left a comment

Choose a reason for hiding this comment

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

Great job, the code looks great! Just left a couple minor comments

@@ -0,0 +1,149 @@

import mysql from 'mysql2';

Choose a reason for hiding this comment

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

None of the files from the week1 assignment should be in this pull request, please remove.

@@ -0,0 +1,19 @@
## What columns violate 1NF?

# food_code and food_description violate 1NF coz they store multiple values.

Choose a reason for hiding this comment

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

This is true but there are 2 other columns that violate 1NF. See if you can spot them

Choose a reason for hiding this comment

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

(Oops I meant to say there is 1 other column, not 2)

Comment on lines 12 to 13


Choose a reason for hiding this comment

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

minor stylistic comment: Similar comment to last time - using lots of whitespace can actually make the code harder to read. The standard approach is to not use more than one line of whitespace to separate logical blocks of code, and not to put whitespace between every little thing. Of course this is subjective but this is most likely the type of feedback you would get in a real life code review

import {createConnection} from './transactions-create-tables.js';


async function transferAmount(fromAccount, toAccount, amount) {

Choose a reason for hiding this comment

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

Nice job making a generic function that can handle a transfer between any two accounts!

@@ -0,0 +1,19 @@
## What columns violate 1NF?

# food_code and food_description violate 1NF coz they store multiple values.

Choose a reason for hiding this comment

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

(Oops I meant to say there is 1 other column, not 2)

# every non-key attribute depends on the entire primay key 2NF
# there is no transitive dependencies 3NF

# member_address column might contain multiple values if a member has more than one address a home and a work address. Storing with comma-separated value would violate 1NF.

Choose a reason for hiding this comment

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

These are all great ideas and ways to avoid potential 1NF violations if they were to occur. But 1NF also says that all the data in a column should be in the same format. There is 1 column that violates this rule in the original table. Which is it?

@@ -0,0 +1,149 @@

Choose a reason for hiding this comment

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

These files are still present. You can simply delete them as part of your next commit. They will still be present on your other branch so you don't need to worry about losing them

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

Successfully merging this pull request may close these issues.

2 participants