DOCSP-32718: deletemany UE code comments #754
Conversation
| const client = new MongoClient(uri); | ||
|
|
||
| async function run() { | ||
| try { | ||
| // Access the movies collection from the sample_mflix database. |
There was a problem hiding this comment.
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.
| await client.close(); | ||
| } | ||
| } | ||
| // Run the program and handle any errors that occur during execution. |
There was a problem hiding this comment.
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."
| const database = client.db("sample_mflix"); | ||
| const movies = database.collection<Movie>("movies"); | ||
|
|
||
| // Delete all documents where the title contains the string "Santa". |
There was a problem hiding this comment.
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."
There was a problem hiding this comment.
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 client = new MongoClient(uri); | ||
|
|
||
| // Define the Movie interface to model the data. |
There was a problem hiding this comment.
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).
* DOCSP-32718: deletemany UE code comments * add ts code * CC comments js file * CC comments ts file
* DOCSP-32718: deletemany UE code comments * add ts code * CC comments js file * CC comments ts file
Pull Request Info
PR Reviewing Guidelines
JIRA - https://jira.mongodb.org/browse/DOCSP-32718
https://preview-mongodbrustagir.gatsbyjs.io/node/DOCSP-32718-deletemany-comments/usage-examples/deleteMany/