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

Beimnet w4 database #52

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

Conversation

beimnet11
Copy link

No description provided.

@ddoyediran ddoyediran self-assigned this Nov 7, 2024

const aggCursor = await collection.aggregate([
{
$match: {
Copy link

@ddoyediran ddoyediran Nov 14, 2024

Choose a reason for hiding this comment

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

Well done @beimnet. We are almost there.
The question is asking us to "Write a function that will return all the information of each continent for a given Year and Age". If you compare your current solutions to the expected output, you will noticed they are not the same. This means in our match, we need to include all the continents (in field named Country) not all the countries. We can update our code to look as follows in the getTotalPopulationByContinent(year, age) and we can remove the commented lines:

...
    const aggCursor = await collection.aggregate([
      {
        $match: {
          Country: {
            $in: [
              "AFRICA",
              "ASIA",
              "EUROPE",
              "LATIN AMERICA AND THE CARIBBEAN",
              "NORTHERN AMERICA",
              "OCEANIA",
            ],
          },
          Year: year,
          Age: age,
        },
      },
      {
        $addFields: {
          TotalPopulation: {
            $add: [{ $toInt: '$M' }, { $toInt: '$F' }],
          },
        },
      },
      // {
      //   $group: {
      //     _id: '$Country',
      //     Year: { $first: '$Year' },
      //     Age: { $first: '$Age' },
      //     M: { $sum: '$M' },
      //     F: { $sum: '$F' },
      //     TotalPopulation: { $sum: '$TotalPopulation' },
      //   },
      // },
      // { $sort: { _id: 1 } },
    ]);
  ....

]);
const result = await aggCursor.toArray();

console.log(result);
Copy link

@ddoyediran ddoyediran Nov 14, 2024

Choose a reason for hiding this comment

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

We can remove this line console.log(result); since we are using another console.log() in line 35.

@beimnet11 beimnet11 requested a review from ddoyediran November 19, 2024 11:11
'OCEANIA',
],
},
year: year,
Copy link

@ddoyediran ddoyediran Nov 22, 2024

Choose a reason for hiding this comment

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

The key should be Capital letter Y as in Year: year, not year: year and same for age as Age: age.
It is always good to try to test our code when we make changes.

const aggCursor = await collection.aggregate([
{
$match: {
country: {

Choose a reason for hiding this comment

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

The key should be Capital letter C as in Country: not country:
It is always good to test our code when we make changes - this way we can know if everything is working as it should.

},
},
{
$group: {
Copy link

@ddoyediran ddoyediran Nov 22, 2024

Choose a reason for hiding this comment

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

I mentioned this in my previous review. We don't need to include line 33 to 42. We can remove it.


const result = await aggCursor.toArray();

console.log(result);

Choose a reason for hiding this comment

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

I mentioned this in my previous review as well, we can remove line 47 console.log(result); since you log the result on line 58.

@beimnet11 beimnet11 requested a review from ddoyediran November 24, 2024 06:15
const uri = process.env.MONGODB_URI;
const client = new MongoClient(uri);

export async function connectToDatabase() {

Choose a reason for hiding this comment

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

Well done @beimnet11, I will approve it now. Congratulations on completing this module - Great job.

One last thing, apologies I keep forgetting this. Since we export both functions connectToDatabase and disconnectFromDatabase on line 23, we don't have to use the export keyword. We can remove the export in front of both functions line 8 & 18. So both will look like:

async function connectToDatabase() {
  try {
    await client.connect();
    console.log('Connected to MongoDB');
    return client.db('databaseWeek4');
  } catch (error) {
    console.error('Failed to connect to MongoDB', error);
    process.exit(1);
  }
}
async function disconnectFromDatabase() {
  await client.close();
  console.log('Disconnected from MongoDB');
}

@ddoyediran
Copy link

Congratulations @beimnet11 on completing the Database module 👏🙌. Rooting for you all. Success with the rest of the programme.

@ddoyediran
Copy link

@beimnet11 Approved now. Thank you

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.

2 participants