Add a student records console application #2
Conversation
|
|
||
| using namespace std; | ||
| // Add URI inside the curly braces in below line. | ||
| static const mongocxx::uri s_Cluster0_uri = mongocxx::uri{ /*Add Uri here ---> */""}; |
There was a problem hiding this comment.
Could you replace this with fetching the value from env variable please ? see https://github.com/mongodb-developer/get-started-cxx/blob/main/cxx/getstarted.cpp#L19
Also, I'd suggest to change the variable name to something related to MongoDB URI with a camel case convention (to match the file)
There was a problem hiding this comment.
I understand it would be a good practice to fetch the URI from an external source generally, but could you help me understand the rationale behind fetching it from env variable rather than just mentioning it here? Bearing in mind, this is a program meant for a new dev to get started asap. Adding a env variable into the mix adds one more step for them. There could be more barriers (for example, students working in labs where they may not have access to create new env variables).
There was a problem hiding this comment.
I understand that this is the simplest way (to type the URI in the code). However there are a number of instances in the past where new developers just copy the example code and ended up committing their Atlas URI on GitHub public repository. Some even think that this method is normal.
There was a problem hiding this comment.
I see, that's a fair enough reason. I'll update this.
There was a problem hiding this comment.
Done.
PS: getenv causes compilation error since MSVC flags it as an unsafe method. Using a safer alternative - _dupenv_s
| const string collName = "StudentCollection"; | ||
|
|
||
| auto dbs = getDatabases(conn); | ||
| // Check if database already exists. |
There was a problem hiding this comment.
This is just a comment, generally in MongoDB databases and collections are created automatically if they don't exist.
| { | ||
| if (doc[key].get_string().value == value) | ||
| { | ||
| collection.update_one(doc, bsoncxx::builder::stream::document{} << "$set" << bsoncxx::builder::stream::open_document << newKey << newValue << bsoncxx::builder::stream::close_document << bsoncxx::builder::stream::finalize); |
There was a problem hiding this comment.
Is there a reason to separate the find and the update operation ?
There was a problem hiding this comment.
Yes - Different method for different responsibilities.
There was a problem hiding this comment.
The function is to update one document, the find() operation is redundant, as update_one() could find the document and update.
There was a problem hiding this comment.
Ah, I misunderstood the earlier comment. I'll cross check this in my code and remove find operation if it's really not needed.
|
|
||
| for (auto&& doc : cursor) | ||
| { | ||
| if (doc[key].get_string().value == value) |
There was a problem hiding this comment.
Is there a reason why there would be a client-side filter here?
There was a problem hiding this comment.
The filter avoids potentially scanning through all documents on the client. It was suggested by C++ engineering lead.
There was a problem hiding this comment.
Line 106: Find document(s) matching key/value. For example rollNo: foo
Line 108: For each of documents returned by the cursor assign to doc. Say there are 2 documents matching rollNo: foo
Line 110: If the string value from doc object with key rollNo is equal to foo print the document
This condition is redundant because all of the documents returned by the cursor should have key=rollNo with the value of foo.
There was a problem hiding this comment.
Removed redundant check.
| // ********************************************** Main ********************************************** | ||
| int main() | ||
| { | ||
| cout << "Hello World!" << endl; |
| } | ||
|
|
||
| // Create an instance. | ||
| mongocxx::instance inst{}; |
There was a problem hiding this comment.
Fix indentation for this code block
This reverts commit 2a969e5.
Will add a permalink to the article (in the file added with this PR) which references this code, once the article is published.