Open a Realm, Call a Function #519

MongoCaleb · 2020-10-06T16:37:07Z

Pull Request Info

Issue JIRA links:

Open a Realm: https://jira.mongodb.org/browse/DOCSP-12487
Call a Function: https://jira.mongodb.org/browse/DOCSP-12476

Docs staging link (requires sign-in on MongoDB Corp SSO):

OaR: https://docs-mongodbcom-staging.corp.mongodb.com/realm/caleb/clientguide/dotnet/open-a-realm.html
CaF: https://docs-mongodbcom-staging.corp.mongodb.com/realm/caleb/clientguide/dotnet/call-a-function.html

nathan-contino-mongo

LGTM with a couple of small nits.

nathan-contino-mongo · 2020-10-06T16:42:34Z

examples/dotnet/Examples/Examples.cs

+            var result = await
+                user.Functions.CallAsync("sum", 2, 40);
+
+            // result.ToInt32() == 42


To me, this comment looks a little out of place in the code-snippet on this page. Could we maybe store the result in a variable and print it out instead?

nathan-contino-mongo · 2020-10-06T16:44:10Z

source/dotnet/open-a-realm.txt

+
+Open a Local (Non-Synced) Realm
+-------------------------------
+When opening a local (non-synced) {+realm+}, you pass a ``RealmConfiguration`` 


Suggested change

When opening a local (non-synced) {+realm+}, you pass a ``RealmConfiguration``

When opening a local (non-synced) {+realm+}, pass a ``RealmConfiguration``

extraneous word, I would probably just trim this.

nathan-contino-mongo · 2020-10-06T16:44:32Z

source/dotnet/open-a-realm.txt

+Close a Realm
+-------------
+There is no need to manually close a {+realm+} with the .NET SDK. When a {+realm+} 
+goes out of scope and is removed from memory, the {+realm+} is closed.


This is great. I wish the other SDKs did this 😢

It's not true unfortunately. The Realm instance implements IDisposable (see https://realm.io/docs/dotnet/latest/#closing-realm-instances) so the general recommendation is to open a Realm instance with a using, unless that instance is going to be long lived (e.g. on the main thread) and you need to continuously read/write data to it as the user interacts with the app. It's true that we have hooks into GC to do that for you, but the issue is that the majority of the memory associated with a Realm instance is native memory which GC doesn't know about. This means that it may take a long time between an instance going out of scope and it being collected because the GC sees it as a very lightweight object and may decide not to run.

nirinchev

Added a few small corrections, but looks good overall.

nirinchev · 2020-10-06T20:23:08Z

source/dotnet/open-a-realm.txt

+
+In the above example, the code shows how to open the {+realm+} *asynchronously* 
+by calling ``GetInstanceAsync``. You can also open a {+realm+} *synchronously* 
+by calling ``GetInstance``, but this may lead to temporary data inconsistencies 


I'm not sure that's very clear - there are no data inconsistencies associated with opening a Realm file synchronously and I don't believe we have strong recommendations against it. In fact, opening a Realm asynchronously is only possible if the device is online and it kind of defeats one of the major selling points for Realm.

I believe the general recommendation we have is to use GetInstanceAsync the first time a user logs in as it's likely that they're connected at this point, but switch to GetInstance after that to ensure offline capabilities.

nirinchev · 2020-10-06T20:25:48Z

examples/dotnet/Examples/Examples.cs

+            user = app.LogInAsync(Credentials.EmailPassword("[email protected]", "foobar")).Result;
+            // :code-block-start: open-synced-realm
+            config = new SyncConfiguration("My Project", user);
+            Realm realm = await Realm.GetInstanceAsync(config);


As mentioned before, let's use var instead of the type name here as it improves readability. It's also consistent with the Swift and Kotlin docs where let and val are used throughout the documentation.

nirinchev · 2020-10-06T20:29:25Z

source/dotnet/open-a-realm.txt

+
+.. note::
+
+   If you specify a local path, that path must already exist on the device.


This is only the case when opening the realm with IsReadOnly = true, which is probably fairly rare. If you're opening a Realm in a read/write mode, the SDK will create the file if it didn't exist before that.

nirinchev · 2020-10-06T20:36:11Z

source/dotnet/open-a-realm.txt

+Close a Realm
+-------------
+There is no need to manually close a {+realm+} with the .NET SDK. When a {+realm+} 
+goes out of scope and is removed from memory, the {+realm+} is closed.


It's not true unfortunately. The Realm instance implements IDisposable (see https://realm.io/docs/dotnet/latest/#closing-realm-instances) so the general recommendation is to open a Realm instance with a using, unless that instance is going to be long lived (e.g. on the main thread) and you need to continuously read/write data to it as the user interacts with the app. It's true that we have hooks into GC to do that for you, but the issue is that the majority of the memory associated with a Realm instance is native memory which GC doesn't know about. This means that it may take a long time between an instance going out of scope and it being collected because the GC sees it as a very lightweight object and may decide not to run.

nirinchev · 2020-10-06T20:38:54Z

examples/dotnet/Examples/Examples.cs

+        {
+            // :code-block-start: callfunc
+            var result = await
+                user.Functions.CallAsync("sum", 2, 40);


It's probably a good idea to showcase the generic overload of CallAsync so that we don't have to work with the raw BsonValue. I believe this would look like var result = await user.Function.CallAsync<int>("sum", 2, 40);

nirinchev

Final nits and clarifications! Thanks for bearing with me and my obsessiveness 🙏

nirinchev · 2020-10-08T08:28:02Z

conf.py

@@ -109,6 +109,7 @@
    'swift-sdk': ('https://docs.mongodb.com/realm-sdks/swift/10.0.0-rc.1/%s', ''),
    'objc-sdk': ('https://docs.mongodb.com/realm-sdks/objc/10.0.0-rc.1/%s', ''),
    'js-sdk': ('https://docs.mongodb.com/realm-sdks/js/latest/%s', ''),
+    'dotnet-sdk': ('https://docs.mongodb.com/realm-sdks/js/latest/%s', ''),


Should this be updated to .../dotnet/latest/...?

nirinchev · 2020-10-08T08:30:37Z

examples/dotnet/Examples/Examples.cs

            {
                Name = "Do this thing",
-                Status = TaskStatus.Open.ToString()
+                Status = dotnet.TaskStatus.Open.ToString()


We probably don't want to use the namespace here?

If it conflicts with something, you can add

using TaskStatus = dotnet.TaskStatus;

next to your using statements to help the compiler resolve the correct type.

nirinchev · 2020-10-08T08:32:12Z

examples/dotnet/Examples/Examples.cs

+        {
+            // :code-block-start: scope
+            config = new SyncConfiguration("My Project", user);
+            using (var realm = await Realm.GetInstanceAsync(config))


You can showcase the new using syntax from C# 8 here and simplify the code a little:

config = new SyncConfiguration(...); using var realm = await Realm.GetInstanceAsync(...); var allTasks = realm.All<RealmTask>();

nirinchev · 2020-10-08T08:36:11Z

source/dotnet/call-a-function.txt

+
+.. note::
+   The ``CallAsync()`` method returns a raw ``BsonValue``, which must be cast to the 
+   expected type, unless you use one of the generic overloads. In the example above,


[super-nit] There's only one generic overload for CallAsync.

nirinchev · 2020-10-08T08:41:21Z

source/dotnet/open-a-realm.txt

+
+.. note::
+
+   The first time a user logs on to your realm app, you should open the realm 


I would use a less compelling verb - i.e. The first time a user logs on to your realm app, it is recommended to open the realm *asynchronously*.

And on a separate note - before the acquisition, the branding instructions stated that we should capitalize Realm when talking about the files - e.g. "Can you send me your Realm file", "To open a synchronized Realm, just pass in a SyncConfiguration with a logged in user", etc. Not sure if that has changed, but might want to sync up with the docs/marketing teams to ensure we're consistent in the usage.

FYI: https://docs.google.com/document/d/1Yc30_-YfVziocquh5jNgBDmTQh0ynrPSyVGSRFma8Jw/edit

Thanks! I hadn't seen this, but super helpful - I just need to untrain my brain to uppercase the local file 😅 But I guess the realm in your realm app needs to be uppercase then?

nirinchev · 2020-10-08T08:53:13Z

source/dotnet/open-a-realm.txt

+
+Scoping the Realm
+-----------------
+The Realm instance implements ``IDisposable``, and you should dispose of a realm 


I would reword this slightly to make it less forceful:

The Realm class implements ``IDisposable`` to clean up the native resources it uses. It is recommended to always dispose of Realm instances after use, especially on background threads. The simplest way is to declare the variable with a `using` or wrap the code that interacts with a Realm in a `using (...)` statement:

Also, might be worth adding a note that disposing of a Realm instance will invalidate all objects associated with that instance, so if the developer needs to use them throughout the lifetime of the app - e.g. for databinding purposes on the main thread - it's safe not to dispose of the Realm. The rule of thumb is "don't dispose on the main thread, dispose on background ones".

MongoCaleb added 9 commits October 2, 2020 16:31

add auth page with code snippets

0462fc4

review

49cdf29

12476 call a function

f13bf6a

12487 open a realm

7eb8242

fix local file error

c88ba41

update unit test?

a1f86d9

Merge branch 'master' of github.com:mongodb/docs-realm into clientguide

b4bce83

moar reviews

91f7dae

merge 12591

9203264

MongoCaleb requested a review from nirinchev October 6, 2020 16:37

nathan-contino-mongo approved these changes Oct 6, 2020

View reviewed changes

nirinchev reviewed Oct 6, 2020

View reviewed changes

MongoCaleb added 4 commits October 6, 2020 13:57

nit and update unit test

a3706f8

more improvements to unit tests

796690b

reviews

06dec75

mrege from master

0bc8f8a

nirinchev approved these changes Oct 8, 2020

View reviewed changes

final review

0d7205b

MongoCaleb merged commit 74f163e into master Oct 8, 2020

MongoCaleb deleted the clientguide branch October 8, 2020 21:47

	When opening a local (non-synced) {+realm+}, you pass a ``RealmConfiguration``
	When opening a local (non-synced) {+realm+}, pass a ``RealmConfiguration``


		.. note::

		If you specify a local path, that path must already exist on the device.


		.. note::

		The first time a user logs on to your realm app, you should open the realm

Open a Realm, Call a Function #519

Open a Realm, Call a Function #519

Uh oh!

Conversation

MongoCaleb commented Oct 6, 2020

Pull Request Info

Issue JIRA links:

Docs staging link (requires sign-in on MongoDB Corp SSO):

Uh oh!

nathan-contino-mongo 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!

nirinchev 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!

nirinchev 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!

nirinchev Oct 8, 2020 • 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!

Uh oh!

nirinchev Oct 8, 2020 •

edited

Loading