Skip to content

DOCSP-32718: deletemany UE code comments #754

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.

Sign up for GitHub

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

Merged
merged 4 commits into from
Aug 31, 2023
Merged

DOCSP-32718: deletemany UE code comments #754

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
a53467b
DOCSP-32718: deletemany UE code comments
rustagir Aug 30, 2023
2d3fd3e
add ts code
rustagir Aug 30, 2023
561e61c
CC comments js file
rustagir Aug 31, 2023
dffefe7
CC comments ts file
rustagir Aug 31, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 12 additions & 1 deletion source/code-snippets/usage-examples/deleteMany.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,21 +1,32 @@
/* Delete multiple documents */

// Import the MongoClient type from the mongodb package.
import { MongoClient } from "mongodb";

// Replace the uri string with your MongoDB deployment's connection string.
const uri = "<connection string uri>";

// Create a new client and connect to MongoDB.
const client = new MongoClient(uri);

async function run() {
try {
// Access the movies collection from the sample_mflix database.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggestion:

Lines 15-16 just get a reference to the collection. I think "access" means interact with it, which is started in line 22 when the deleteMany operation is called on the collection.

const database = client.db("sample_mflix");
const movies = database.collection("movies");
// Query for all movies with a title containing the string "Santa"

// Create a filter for movies with a title containing the string "Santa".
const query = { title: { $regex: "Santa" } };

// Delete all documents that match the preceding query filter.
const result = await movies.deleteMany(query);

// Print the number of deleted documents.
console.log("Deleted " + result.deletedCount + " documents");
} finally {
// Close the client after the operation completes.
await client.close();
}
}
// Run the program and handle any errors that occur during execution.
run().catch(console.dir);
12 changes: 12 additions & 0 deletions source/code-snippets/usage-examples/deleteMany.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,22 +1,34 @@
/* Delete multiple documents */

// Import the MongoClient type from the mongodb package.
import { MongoClient } from "mongodb";

// Replace the uri string with your MongoDB deployment's connection string.
const uri = "<connection string uri>";

// Create a new client and connect to MongoDB.
const client = new MongoClient(uri);

// Define the Movie interface to model the data.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggestion:

I'm open to any discussion, but I think some of the information is self-apparent. I think "Create a sample interface" contains enough information.

Also, this code isn't used in this example, so I think it could be good to remove (feel free to ticket if it's not trivial to fix and check).

interface Movie {
title: string;
}

async function run() {
try {
// Access the movies collection from the sample_mflix database.
const database = client.db("sample_mflix");
const movies = database.collection<Movie>("movies");

// Delete all documents where the title contains the string "Santa".
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggestion:
Instead of "where" (location), I think it could be good to use "in which" or to describe the matching.

While I think it can be helpful to include values, since this is a regular expression, it might get awkward to try to reproduce this language for a more complex regular expression. I think it could be more general since the value is apparent and self-explanatory in the code.

I think it could be helpful to skip the comment "Access the movies collection..." and reference the collection in this comment instead since this is closer to the usage of the collection. E.g.

"Delete all documents that match the specified regular expression in the title field from the "movies" collection."

Copy link
Contributor

Choose a reason for hiding this comment

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

I think this is also inconsistent with the other comment in the JS file even though the code is the same:
"// Delete all documents that match the preceding query filter."

const result = await movies.deleteMany({ title: { $regex: "Santa" } });

// Print the number of deleted documents.
console.log("Deleted " + result.deletedCount + " documents");
} finally {
// Close the client after the operation completes.
await client.close();
}
}
// Run the program and handle any errors that occur during execution.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggestion:
I think this only prints the exception rather than handles them. I think "handle [an] error" usually refers to the control logic (rethrow, close resources, etc.). I think printing could technically be handling the error, but probably not the first thought. I think this could be more accurate and concise:

"Run the program and print any thrown exceptions."

run().catch(console.dir);