DOCSP-33345: Java comments pt. 5 #473

norareidy · 2023-10-10T20:15:10Z

Pull Request Info

JIRA - https://jira.mongodb.org/browse/DOCSP-33345
Staging:
CurrentAPI - https://preview-mongodbnorareidy.gatsbyjs.io/java/DOCSP-33345-java-comments-5/faq/
Cursor - https://preview-mongodbnorareidy.gatsbyjs.io/java/DOCSP-33345-java-comments-5/fundamentals/crud/read-operations/cursor/
Delete - https://preview-mongodbnorareidy.gatsbyjs.io/java/DOCSP-33345-java-comments-5/fundamentals/crud/write-operations/delete/
Filters - https://preview-mongodbnorareidy.gatsbyjs.io/java/DOCSP-33345-java-comments-5/fundamentals/builders/filters/
Geo - https://preview-mongodbnorareidy.gatsbyjs.io/java/DOCSP-33345-java-comments-5/fundamentals/crud/read-operations/geo/

Self-Review Checklist

Is this free of any warnings or errors in the RST?
Did you run a spell-check?
Did you run a grammar-check?
Are all the links working?

source/includes/fundamentals/code-snippets/CurrentAPI.java

ccho-mongodb · 2023-10-11T20:19:08Z

source/includes/fundamentals/code-snippets/CurrentAPI.java

        c.example1();
        c.example2();

    }

    private void example1() {
+        // Connect to a MongoDB instance with the current API


Issue:
Use the indicative mood (verb form that makes a statement). See https://docs.google.com/document/d/1Bnhh1OsGRUuXo_6yVUASGWiSX6sWuqrEFxPP6sLZDHM/edit for examples.
Also, I think "by using" is suitable and more compliant with the style guide.

Suggestion:

Maybe something like the following would be accurate and follow the style guide:
"Connects to MongoDB and retrieves a document by using the non-legacy API"

ccho-mongodb · 2023-10-11T20:22:41Z

source/includes/fundamentals/code-snippets/CurrentAPI.java

        // start current-api-example
        MongoClient client = MongoClients.create(URI);
        MongoDatabase db = client.getDatabase(DATABASE);
        MongoCollection<Document> col = db.getCollection(COLLECTION);
+
+        // Find and print a document in your collection to test your connection


Issue:
Use the indicative mood (verb form that makes a statement).

Suggestion:

I think it's not necessary to mention the "test your connection" part since it doesn't explicitly run any tests. All operations would implicitly test the connection given that they would throw a runtime exception if unable to connect, similar to this operation.

I think the following might be an accurate summary comment that uses the indicative mood:
"Prints the first document retrieved from the collection in JSON format."

ccho-mongodb

Looks pretty good -- I appreciate that you referenced the code in which each snippet was included and provided the staging links for each page.
I think the feedback I provided is mostly repeat advice, but wanted to make sure I provided solutions.

ccho-mongodb · 2023-10-11T21:50:22Z

source/includes/fundamentals/code-snippets/CurrentAPI.java

        c.example1();
        c.example2();

    }

    private void example1() {
+        // Connects to a MongoDB instance with the current API


Issue:
From this comment, I think "by using" is suitable and more compliant with the style guide.

Suggestion:
I think the same suggestion as proposed in the linked comment applies.

ccho-mongodb · 2023-10-11T21:51:35Z

source/includes/fundamentals/code-snippets/CurrentAPI.java

        // start current-api-example
        MongoClient client = MongoClients.create(URI);
        MongoDatabase db = client.getDatabase(DATABASE);
        MongoCollection<Document> col = db.getCollection(COLLECTION);
+
+        // Finds and prints a document in your collection


Suggestion:
From this comment, I think there's a more accurate and concise way to describe the code.

ccho-mongodb · 2023-10-11T21:58:36Z

source/includes/fundamentals/code-snippets/Geo.java

        collection.find(query)
                .projection(projection)
                .forEach(doc -> System.out.println(doc.toJson()));
        // end findExample
    }

    private void rangeExample() {
+        // Accesses the "theaters" collection in the "sample_mflix" database
        MongoDatabase database = mongoClient.getDatabase("sample_mflix");
        MongoCollection<Document> collection = database.getCollection("theaters");
        // begin rangeExample

        // code to set up your mongo collection ...


Suggestion:
While this is not your code comment, could it be updated to either be consistent with other instances ("// ") or be replaced by a new imperative comment such as "// Add your MongoCollection setup code here"

Applies to all instances.

ccho-mongodb · 2023-10-11T22:12:08Z

source/includes/fundamentals/code-snippets/CurrentAPI.java

        Document doc = col.find().first();
        System.out.println(doc.toJson());
        // end current-api-example
        client.close();
    }

    private void example2() {
+        // Sets a write concern on your client with the current API


Suggestion:

I think "sets a write concern" describes only a part of what the code snippet accomplishes. I also think there's always a write concern setting (whether a number or "majority"), so it would be more helpful to explain which one.

Suggested change

// Sets a write concern on your client with the current API

// Creates a MongoClient that requests write acknowledgement from at least one node.

ccho-mongodb · 2023-10-11T22:14:38Z

source/includes/fundamentals/code-snippets/Cursor.java

@@ -59,27 +60,31 @@ public static void main(String [] args){
    }

    private void forEachIteration(){
+        // Iterates through your cursor results


Suggestion:
Similar to this comment, I think this should focus on the functionality instead of the code itself (which most of your comments achieve successfully).

Suggested change

// Iterates through your cursor results

// Prints all the documents in the collection in JSON format

or

Suggested change

// Iterates through your cursor results

// Prints all the document in the collection as JSON

ccho-mongodb · 2023-10-12T01:05:46Z

source/includes/fundamentals/code-snippets/Filters.java

        Bson geoWithinComparison = geoWithin("coordinates", square);
        collection.find(geoWithinComparison).forEach(doc -> System.out.println(doc.toJson()));
        // end geoWithinComparison
    }

    private void preview(){
+        // Uses an empty filter to print all documents in the collection


Suggestion:
I think this should be omitted since it's setup code meant only for tech writers

ccho-mongodb · 2023-10-12T01:05:59Z

source/includes/fundamentals/code-snippets/Filters.java

@@ -144,6 +157,8 @@ private void preview(){
    private void setupPaintCollection() {

        List<Document> filterdata = new ArrayList<>();
+
+        // Set up the "vendor" field for your documents


Suggestion:
I think this should be omitted since it's setup code meant only for tech writers

ccho-mongodb · 2023-10-12T01:06:16Z

source/includes/fundamentals/code-snippets/Filters.java

@@ -153,6 +168,7 @@ private void setupPaintCollection() {
        String [] p7a = {"B", "C"};
        String [] p8a = {"A", "D"};

+        // Creates a series of documents and inserts them into the "paint" collection


Suggestion:
I think this should be omitted since it's setup code meant only for tech writers

ccho-mongodb · 2023-10-12T01:06:30Z

source/includes/fundamentals/code-snippets/Filters.java

@@ -178,6 +194,7 @@ private void setupPaintCollection() {
    private void setupBinaryCollection(){
        List<Document> filterdata = new ArrayList<>();

+        // Creates a series of documents and inserts them into the "binary" collection


Suggestion:
I think this should be omitted since it's setup code meant only for tech writers

ccho-mongodb · 2023-10-12T01:06:39Z

source/includes/fundamentals/code-snippets/Filters.java

@@ -194,6 +211,7 @@ private void setupPointsCollection(){

        List<Document> filterdata = new ArrayList<>();

+        // Creates a series of documents and inserts them into the "points" collection


Suggestion:
I think this should be omitted since it's setup code meant only for tech writers

ccho-mongodb

Looks good. Identified a few issues and suggestions to address before merging.

ccho-mongodb · 2023-10-26T01:17:27Z

source/includes/fundamentals/code-snippets/CurrentAPI.java

        c.example1();
        c.example2();

    }

    private void example1() {
+        // Connects to MongoDB and retrieves a document by using the non-legacy API


Suggestion:

I think "non-legacy API" could be confusing since it's not a common way to describe an API. I think it would only be used sparingly in which it was directly adjacent to the legacy API and that extra emphasis needed to be placed on the contrast.

Suggested change

// Connects to MongoDB and retrieves a document by using the non-legacy API

// Connects to MongoDB and retrieves a document

ccho-mongodb · 2023-10-26T03:16:19Z

source/includes/fundamentals/code-snippets/CurrentAPI.java

        // start current-api-example
        MongoClient client = MongoClients.create(URI);
        MongoDatabase db = client.getDatabase(DATABASE);
        MongoCollection<Document> col = db.getCollection(COLLECTION);
+
+        // Prints the first document retrieved from the collection as JSON 


Issue:
I think it could be good to only feature one Summary-type comment per method.

Suggestion:
I think omitting the Summary-type comment on line 29 ("Connects to MongoDB and retrieves a document...") would work well.

ccho-mongodb · 2023-10-26T05:39:34Z

source/includes/fundamentals/code-snippets/Delete.java

        // begin deleteManyExample
        Bson filter = Filters.eq("qty", 0);
        collection.deleteMany(filter);
        // end deleteManyExample
    }

    private void findOneAndDeleteExample(){
+        // Deletes the first document with a "color" value of "purple" and prints the deleted document
        // begin findOneAndDeleteExample
        Bson filter = Filters.eq("color", purple);


Issue:
Io think this code won't compile because "purple" isn't a defined symbol.

Suggestion:
While it's not a code comment, I think it would be good to correct this to the string "purple".

ccho-mongodb · 2023-10-26T05:41:10Z

source/includes/fundamentals/code-snippets/Delete.java

        // begin deleteOneExample
        Bson filter = Filters.eq("color", "yellow");
        collection.deleteOne(filter);
        // end deleteOneExample
    }
    private void preview(boolean drop){
+        // Prints the results of a find operation as JSON


Suggestion:
I think it could be good to keep this consistent with other comments on similar code.

ccho-mongodb · 2023-10-26T05:42:47Z

source/includes/fundamentals/code-snippets/Geo.java

@@ -46,12 +46,19 @@ public void go() {
    private void nearExample() {
        // begin findExample

-        // code to set up your mongo client ...
+        // Add your MongoCollection setup code here


Issue:
As the prior version of this comment mentions, this should be the "MongoClient" and not "MongoCollection".

Suggestion:

Suggested change

// Add your MongoCollection setup code here

// Add your MongoClient setup code here

ccho-mongodb · 2023-10-26T05:45:24Z

source/includes/fundamentals/code-snippets/Geo.java

        Point centralPark = new Point(new Position(-73.9667, 40.78));
+
+        // Creates a query that matches all theaters between 5,000 and 10,000 meters from the specified Point


Issue:
By itself, this query does not match theaters. Instead, it just matches all points within a range from the starting coordinates.

Suggestion:

Suggested change

// Creates a query that matches all theaters between 5,000 and 10,000 meters from the specified Point

// Creates a query that matches all locations between 5,000 and 10,000 meters from the specified Point

ccho-mongodb · 2023-10-26T05:47:51Z

source/includes/fundamentals/code-snippets/Geo.java

@@ -63,13 +70,20 @@ private void rangeExample() {
        MongoCollection<Document> collection = database.getCollection("theaters");
        // begin rangeExample

-        // code to set up your mongo collection ...
+        // Add your MongoCollection setup code here


Suggestion:
I think it would be good to include a linebreak between two separate comments so it can be read and understood as a separate comment. I think the blank line after the Marker comment ("begin rangeExample") is not necessary.

ccho-mongodb · 2023-10-26T05:49:56Z

source/includes/fundamentals/code-snippets/Geo.java

@@ -63,13 +70,20 @@ private void rangeExample() {
        MongoCollection<Document> collection = database.getCollection("theaters");
        // begin rangeExample

-        // code to set up your mongo collection ...
+        // Add your MongoCollection setup code here
+        // Creates a Polygon instance for a geolocation query


Suggestion:
I think it would be better to omit the part of this comment that just describes the code ("Polygon instance").

E.g.
"Creates a set of points that defines the bounds of a geospatial shape"

ccho-mongodb · 2023-10-26T05:52:10Z

source/includes/fundamentals/code-snippets/Geo.java

        Bson projection = fields(include("location.address.city"), excludeId());
+
+        // Prints the projected field of the results from the geolocation query as JSON


Issue:
I think the correct term is geospatial query as described by the Server docs.

Suggestion:
I think it would be good to use the correct terminology in all instances.

ccho-mongodb · 2023-10-26T05:55:22Z

source/includes/fundamentals/code-snippets/Geo.java

        Bson projection = fields(include("location.address.city"), excludeId());
+
+        // Creates a query that matches documents containing "location.geo" values within the specified Polygon


Suggested change

// Creates a query that matches documents containing "location.geo" values within the specified Polygon

// Creates a query that matches documents containing "location.geo" values within the specified shape

* DOCSP-33345: Java comments pt. 5 * indicative mood * review feedback * sheet syntax * final feedback (cherry picked from commit e14a975)

DOCSP-33345: Java comments pt. 5

6f37512

ccho-mongodb reviewed Oct 11, 2023

View reviewed changes

indicative mood

37d6011

ccho-mongodb suggested changes Oct 12, 2023

View reviewed changes

norareidy force-pushed the DOCSP-33345-java-comments-5 branch from 220a552 to 37d6011 Compare October 16, 2023 14:42

norareidy added 2 commits October 16, 2023 11:10

review feedback

2ab966b

sheet syntax

ee6d225

norareidy requested a review from ccho-mongodb October 19, 2023 14:42

ccho-mongodb approved these changes Oct 26, 2023

View reviewed changes

final feedback

cd22bf6

norareidy merged commit e14a975 into mongodb:master Oct 27, 2023

norareidy deleted the DOCSP-33345-java-comments-5 branch October 27, 2023 17:34

norareidy added a commit that referenced this pull request Oct 27, 2023

DOCSP-33345: Java comments pt. 5 (#473)

9b97fb5

* DOCSP-33345: Java comments pt. 5 * indicative mood * review feedback * sheet syntax * final feedback (cherry picked from commit e14a975)

norareidy added a commit that referenced this pull request Oct 27, 2023

DOCSP-33345: Java comments pt. 5 (#473)

7111bf9

* DOCSP-33345: Java comments pt. 5 * indicative mood * review feedback * sheet syntax * final feedback (cherry picked from commit e14a975)

norareidy added a commit that referenced this pull request Oct 27, 2023

DOCSP-33345: Java comments pt. 5 (#473)

a21b946

* DOCSP-33345: Java comments pt. 5 * indicative mood * review feedback * sheet syntax * final feedback (cherry picked from commit e14a975)

norareidy added a commit that referenced this pull request Oct 27, 2023

DOCSP-33345: Java comments pt. 5 (#473)

7de79d9

* DOCSP-33345: Java comments pt. 5 * indicative mood * review feedback * sheet syntax * final feedback (cherry picked from commit e14a975)

norareidy added a commit that referenced this pull request Oct 27, 2023

DOCSP-33345: Java comments pt. 5 (#473)

2b5dc33

* DOCSP-33345: Java comments pt. 5 * indicative mood * review feedback * sheet syntax * final feedback (cherry picked from commit e14a975)

norareidy added a commit that referenced this pull request Oct 27, 2023

DOCSP-33345: Java comments pt. 5 (#473)

916ddef

* DOCSP-33345: Java comments pt. 5 * indicative mood * review feedback * sheet syntax * final feedback (cherry picked from commit e14a975)

norareidy added a commit that referenced this pull request Oct 27, 2023

DOCSP-33345: Java comments pt. 5 (#473)

c62e04d

* DOCSP-33345: Java comments pt. 5 * indicative mood * review feedback * sheet syntax * final feedback (cherry picked from commit e14a975)

	// Sets a write concern on your client with the current API
	// Creates a MongoClient that requests write acknowledgement from at least one node.

	// Iterates through your cursor results
	// Prints all the documents in the collection in JSON format

	// Iterates through your cursor results
	// Prints all the document in the collection as JSON

		@@ -194,6 +211,7 @@ private void setupPointsCollection(){

		List<Document> filterdata = new ArrayList<>();

		// Creates a series of documents and inserts them into the "points" collection

	// Connects to MongoDB and retrieves a document by using the non-legacy API
	// Connects to MongoDB and retrieves a document

	// Add your MongoCollection setup code here
	// Add your MongoClient setup code here

		Point centralPark = new Point(new Position(-73.9667, 40.78));

		// Creates a query that matches all theaters between 5,000 and 10,000 meters from the specified Point

	// Creates a query that matches all theaters between 5,000 and 10,000 meters from the specified Point
	// Creates a query that matches all locations between 5,000 and 10,000 meters from the specified Point

		Bson projection = fields(include("location.address.city"), excludeId());

		// Prints the projected field of the results from the geolocation query as JSON

		Bson projection = fields(include("location.address.city"), excludeId());

		// Creates a query that matches documents containing "location.geo" values within the specified Polygon

DOCSP-33345: Java comments pt. 5 #473

DOCSP-33345: Java comments pt. 5 #473

Uh oh!

Conversation

norareidy commented Oct 10, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Pull Request Info

Self-Review Checklist

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ccho-mongodb left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ccho-mongodb Oct 11, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ccho-mongodb left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

norareidy commented Oct 10, 2023 •

edited

Loading

ccho-mongodb Oct 11, 2023 •

edited

Loading