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

Assignment w2 HANNA MELNYK #13

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

Conversation

hanna-melnyk
Copy link

No description provided.

@rafaelhdr
Copy link

Exercises are going very well. Easy to read and review.

But exercise 4 is failing. It is something very small that is raising an error. Could you investigate the issue, please?

When we run it, we get this:

node exercise_4.js
An error occurred: TypeError: Cannot read properties of undefined (reading 'query')
    at file:///.../connection_query.js:20:20
    at new Promise (<anonymous>)
    at useDatabase (file:///home/rafaelhdr/hyf/hanna/Week2/connection_query.js:15:12)
    at exerciseFour (file:///home/rafaelhdr/hyf/hanna/Week2/exercise_4.js:148:15)
    at file:///home/rafaelhdr/hyf/hanna/Week2/exercise_4.js:162:5
    at ModuleJob.run (node:internal/modules/esm/module_job:217:25)
    at async ModuleLoader.import (node:internal/modules/esm/loader:316:24)
    at async loadESM (node:internal/process/esm_loader:34:7)
    at async handleMainPromise (node:internal/modules/run_main:66:12)

@rafaelhdr rafaelhdr self-assigned this Aug 22, 2024
@hanna-melnyk
Copy link
Author

hanna-melnyk commented Aug 22, 2024

Exercises are going very well. Easy to read and review.

But exercise 4 is failing. It is something very small that is raising an error. Could you investigate the issue, please?

When we run it, we get this:

node exercise_4.js
An error occurred: TypeError: Cannot read properties of undefined (reading 'query')
    at file:///.../connection_query.js:20:20
    at new Promise (<anonymous>)
    at useDatabase (file:///home/rafaelhdr/hyf/hanna/Week2/connection_query.js:15:12)
    at exerciseFour (file:///home/rafaelhdr/hyf/hanna/Week2/exercise_4.js:148:15)
    at file:///home/rafaelhdr/hyf/hanna/Week2/exercise_4.js:162:5
    at ModuleJob.run (node:internal/modules/esm/module_job:217:25)
    at async ModuleLoader.import (node:internal/modules/esm/loader:316:24)
    at async loadESM (node:internal/process/esm_loader:34:7)
    at async handleMainPromise (node:internal/modules/run_main:66:12)

Rafael, thanks for the feedback!
I've changed the exercise_4.js file. Hope it works now.

@rafaelhdr
Copy link

So quick. Thanks for checking.

But the issue is not on the require.main

Try to compare how you are calling the SQL methods (try checking how you are calling the useDatabase in exercise 4 with the exercise 3). It is really something very small.

Copy link

@rafaelhdr rafaelhdr left a comment

Choose a reason for hiding this comment

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

It is really good. Congratulations. I just wanted to share a suggestion, but the query provided works good.

const getSumOfPapersByFemaleAuthors = (connection) => {
return new Promise((resolve, reject) => {
const query = `
SELECT

Choose a reason for hiding this comment

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

I would recommend to do this query like this:

            SELECT 
                COUNT(author_papers.paper_id) AS total_papers
            FROM 
                author_papers
            LEFT JOIN 
                authors ON author_papers.author_id = authors.author_id 
            WHERE authors.gender = 'Female' 

If we have a index for gender, the performance would be better because we don't need to go through all rows.

But your answer is correct. I would just recommend use WHERE if possible.

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

Successfully merging this pull request may close these issues.

3 participants