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

(DOCSP-39502): Consolidate Define an Object Model page #3314

Merged
merged 36 commits into from
Jul 19, 2024

Conversation

dacharyc
Copy link
Collaborator

@dacharyc dacharyc commented Jun 27, 2024

Pull Request Info - SDK Docs Consolidation

Jira ticket: https://jira.mongodb.org/browse/DOCSP-39502

Staged Page

Page Source

Add links to every SDK's pages where you got the SDK-specific information:

PR Author Checklist

Before requesting a review for your PR, please check these items:

  • Open the PR against the feature-consolidated-sdk-docs branch instead of master
  • Tag the consolidated page for:
    • genre
    • meta.keywords
    • meta.description

Naming

Links and Refs

  • Create new consolidated SDK ref targets starting with "_sdks-" for relevant sections
  • Remove or update any SDK-specific refs to use the new consolidated SDK ref targets
  • Update any Kotlin API links to use the new Kotlin SDK roles

Content

  • Shared code boxes have snippets or placeholders for all 9 languages
  • API description sections have API details or a generic placeholder for all 9 languages
  • Check related pages for relevant content to include
  • Create a ticket for missing examples in each relevant SDK: Consolidation Gaps epic

Reviewer Checklist

As a reviewer, please check these items:

  • Shared code example boxes contain language-specific snippets or placeholders for every language
  • API reference details contain working API reference links or generic content
  • Realm naming/language has been updated
  • All relevant content from individual SDK pages is present on the consolidated page

@dacharyc dacharyc added the merge to feature branch Unreleased feature - do not merge to Master label Jun 27, 2024
Copy link

github-actions bot commented Jun 27, 2024

Readability for Commit Hash: 1e30fb0

You can see any previous Readability scores (if they exist) by looking
at the comment's history.

Readability scores for changed documents:

  • source/sdk/model-data: Grade Level: 25.8, Reading Ease: -51.88
  • source/sdk/model-data/object-models: Grade Level: 9.0, Reading Ease: 56.55
  • source/sdk/model-data/property-types: Grade Level: 8.9, Reading Ease: 40.72
  • source/sdk/sync/event-library: Grade Level: 6.9, Reading Ease: 67.35
  • source/sdk/files/configure-and-open: Grade Level: 6.8, Reading Ease: 67.76
  • source/sdk/crud/read: Grade Level: 8.6, Reading Ease: 52.36

For Grade Level, aim for 8 or below.

For Reading Ease scores, aim for 60 or above:

Score Difficulty
90-100 Very Easy
80-89 Easy
70-79 Fairly Easy
60-69 Medium
50-59 Fairly Hard
30-49 Hard
0-29 Very Hard

For help improving readability, try Hemingway App.

Copy link
Collaborator

@krollins-mdb krollins-mdb left a comment

Choose a reason for hiding this comment

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

Added some suggestions for the JS object model info.

Comment on lines 2 to 3
(like most of the examples on this page). Or you can define models as
JavaScript objects.
Copy link
Collaborator

Choose a reason for hiding this comment

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

After talking with the JS SDK team, it makes sense to simplify the approach to only using classes. With the caveat that we never use new to construct a new instance. Instead, we define a model with a class, then create objects use an object literal. Like this:

class Book extends Realm.Object {
  // Don't forget to declare these.
  title!: string;
  price!: number;

  static schema: ObjectSchema = {
    name: "Book",
    properties: {
      title: "string",
      price: "double",
    },
  };
}

const book = realm.write(() => {
  return realm.create(Book, { title: "My Title", price: 20.25 });
});

// TS knows about these fields from the declarations in the model.
book.title;
book.price;

Passing the class into realm.create, followed by an object literal, gives us a better dev experience in TypeScript. It doesn't provide any benefits in JavaScript, but we should use the same approach in both for consistency.

Copy link
Collaborator

Choose a reason for hiding this comment

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

We should note that you shouldn't use new with class-based models for a few reasons:

  • It calls realm.create(), so must always be used in a write transaction
  • It doesn't play well with embedded objects

It really comes down to when objects are just objects and when they are managed objects. Calling new automatically creates a managed object.

Not calling new lets you create unmanaged objects, then pass them and the class model to realm.create() when it makes sense to do so.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Got it! Thanks for clarifying these details with the JS team.

I've updated the wording here and in the TS description. Can you take a look and make sure this accurately communicates the important points?

I do worry that we're now saying something in the docs that is not consistent with all of our code examples. But I guess that's a problem to address in the consolidation gaps epic.

Copy link
Collaborator

Choose a reason for hiding this comment

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

We seem to mostly adhere to the guidance - at least on this page. The Node.js examples definitely need work. They haven't been modernized/standardized in a long time (since before I started at MongoDB).

Comment on lines 9 to 13
If you do define a model that does not extend ``Realm.Object``, you cannot pass
the model directly to the database. Instead, you pass only the schema to the
database that manages the object.

.. TODO: Provide more info about why you would choose one of these approaches over the other
Copy link
Collaborator

Choose a reason for hiding this comment

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

Based on my comment above, we don't recommend this and should probably avoid talking about it.

Comment on lines +8 to +12
When the SDK model does *not* extend ``Realm.Object``, as when it is an
embedded object, you pass only the object's schema to the database ``schema``
list. You cannot pass the object itself directly to the database.

.. TODO: Verify that this is accurate
Copy link
Collaborator

Choose a reason for hiding this comment

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

Let's leave this approach out of the docs.

@dacharyc dacharyc marked this pull request as ready for review July 10, 2024 16:01
Copy link
Collaborator

@krollins-mdb krollins-mdb left a comment

Choose a reason for hiding this comment

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

Requesting changes mainly for some remaining "realm" stuff. I had a question in particular about "Realm object" that will affect a good number of my suggestions.

@dacharyc, thank you for doing this difficult and delicate work! Having everything together like this makes it much easier to understand how to work with objects. My brain is spaghetti after reading it. It must have been such a bear to put together.

A higher-level question: do we want to introduce a generic managed vs not managed objects section that talks about the different between language objects and realm objects?

.. tab::
:tabid: typescript

.. include:: /includes/api-details/javascript/model-data/object-models-object-schema-js-ts-description.rst

.. _sdks-define-objects:

Define an SDK Object Model
Copy link
Collaborator

Choose a reason for hiding this comment

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

This section feels odd because it's the heading is exact same as the page title, but it's kind of just there without any words to introduce the section or explain why there's a code example here.

Feels like this was meant to be integrated into another section?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Hah. Good catch. It only looks like that because I forgot to go back and add descriptions for JavaScrpt/TypeScript after our convos with engineering - all of the other languages have text here. This is "How to Define an Object Model," but Cory and I talked about it, and we don't really use "How to" in our heading titles.

I will add in the JavaScript/TypeScript descriptions, and will experiment with tweaking the headings so they're not exactly the same.

#. At a minimum, add the two fields required by the GeoJSON spec:

- A field of type ``IList<double>`` that maps to a "coordinates" (case sensitive)
property in the realm schema.
Copy link
Collaborator

Choose a reason for hiding this comment

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

"realm" should probably be changed to "database", right?

Use the Embedded Class
``````````````````````

You then use the custom GeoPoint class in your Realm object model, as shown in
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
You then use the custom GeoPoint class in your Realm object model, as shown in
You then use the custom GeoPoint class in your SDK object model, as shown in

@@ -0,0 +1,8 @@
In C#, to define a Realm object, inherit from the the
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
In C#, to define a Realm object, inherit from the the
In C#, to define an SDK object, inherit from the the

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I think in this case, we are specifically talking about Realm object versus the other object types (Asymmetric, Embedded) so this is an appropriate Realm usage.


Then, use the generated ``RealmObject`` model in your application code.

The following example shows how to model an embedded object in a Realm schema.
Copy link
Collaborator

Choose a reason for hiding this comment

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

"Realm schema" to "database schema"?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Hmm. This one is tricky - I'm going to go with "Realm object schema" because we're specifically talking about the schema for a Realm object.

@@ -0,0 +1,2 @@
In Swift, to define a Realm object, create a class that inherits from
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
In Swift, to define a Realm object, create a class that inherits from
In Swift, to define an SDK object, create a class that inherits from

#. At a minimum, add the two fields required by the GeoJSON spec:

- A field of type ``double[]`` that maps to a "coordinates" (case sensitive)
property in the realm schema.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
property in the realm schema.
property in the database schema.

The SDK provides three specific object types, plus support for using an object
with geospatial data:

- **Realm object**: the base object type used by the SDK.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Are we sticking with Realm for this use case? Could we use "managed object" instead? The decision here would impact my suggestions in other places to replace "realm object".

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I think it's important to stick with Realm for this use case, since literally all the APIs and API docs refer to it that way. As it's just the name of a specific object type in this context, I don't think this is inconsistent with Realm database rebranding.

.. _sdks-define-special-property-types:

Define Special Property Types
-----------------------------
Define Special Property Behaviors
Copy link
Collaborator

Choose a reason for hiding this comment

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

I know it's from the old content, but are these really "special"? I think "Define Property Behaviors" would work better.

@@ -0,0 +1,3 @@
In the Kotlin SDK, to define an asymmetric object type, create a Kotlin class
that implements the :kotlin-sdk:`AsymmetricRealmObject
<io.realm.kotlin.types/-asymmetric-realm-object/index.html>` interface:
Copy link
Collaborator

Choose a reason for hiding this comment

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

I'm getting a 403 on this link. However, I can navigate to the right page and copy-pasting from there works. It's the exact same URL, so this could absolutely be a me thing.

Resolved URL that works for me: https://www.mongodb.com/docs/realm-sdks/kotlin/latest/library-sync/io.realm.kotlin.types/-asymmetric-realm-object/index.html

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Good catch! This was my error - this link should use the :kotlin-sync-sdk: role, not the :kotlin-sdk: role. Updating!

Copy link
Collaborator

@krollins-mdb krollins-mdb left a comment

Choose a reason for hiding this comment

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

Excellent work, @dacharyc! Thank you for putting so much thought and effort into this.

@dacharyc dacharyc merged commit ebc8e9d into mongodb:feature-consolidated-sdk-docs Jul 19, 2024
6 checks passed
@dacharyc dacharyc deleted the DOCSP-39502 branch July 19, 2024 15:57
@docs-builder-bot
Copy link

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
merge to feature branch Unreleased feature - do not merge to Master
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants