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-14573): set data type #1079

Conversation

mohammadhunan-dev
Copy link
Contributor

@mohammadhunan-dev mohammadhunan-dev commented May 13, 2021

Pull Request Info

Jira

Staged Changes (Requires MongoDB Corp SSO)

Review Guidelines

REVIEWING.md

@mohammadhunan-dev mohammadhunan-dev changed the base branch from master to new-data-types-node-rn May 13, 2021 06:57
@mohammadhunan-dev mohammadhunan-dev changed the title WIP - (DOCSP-14573): set data type (DOCSP-14573): set data type May 13, 2021
@mohammadhunan-dev mohammadhunan-dev requested review from kneth and fronck May 13, 2021 13:38
Copy link
Collaborator

@MongoCaleb MongoCaleb left a comment

Choose a reason for hiding this comment

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

Overall comment: I have no idea WHY I would use a set over an array. Perhaps JS developers already know that? Also, there is no section on how to retrieve a value from a set, and I assume that's because you can't -- you have to iterate? Is it worth any additional info about that?

--------
A {+service-short+} set is a special object that allows you to store a
Copy link
Collaborator

Choose a reason for hiding this comment

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

"special object" sounds weird and doesn't really tel us anything. Since everyone is familiar with Arrays, perhaps a start like this?

A {+service-short+} set is an object that stores a collection of values. A Set is similar to an Array, except:

  • Each value in a Set must be unique,
  • Arrays are ordered by index, while sets are iterable and unordered (this isn't quite right...)

Copy link
Contributor Author

@mohammadhunan-dev mohammadhunan-dev May 18, 2021

Choose a reason for hiding this comment

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

I'm down for being a bit clearer on the advantages of Set. I do want to keep it a bit more subtle and not necessarily bring up array for the following reasons:

  • JavaScript developers should probably know the differences between set and array. Explaining the differences between the two would be explaining JS, not explaining how to use Realm with JS. For those that aren't familiar with the differences, I've linked out to the mdn docs on set.

As for showing when to use set, I've added some info in the intro, the new intro reads:

"A Realm Set is a special object that allows you to store a collection of unique values. Realm Sets are based on JavaScript Sets, but can only contain values of a single type and can only be modified within a write transaction. Sets allow you to perform math operations such as finding the union, intersection, or difference between two sets. To learn more about performing these operations, see the MDN docs for Implementing basic set operations."

Copy link

Choose a reason for hiding this comment

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

The JS Set does not currently support the math operations. Its implementation has been kept as close as possible to JS's own set type.

--------
A {+service-short+} set is a special object that allows you to store a
collection of unique values. {+service-short+} sets work similarly to JavaScript
Copy link
Collaborator

Choose a reason for hiding this comment

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

sets work similarly to JavaScript
to
sets are based on JavaScript Sets, but are limited to a single value type and can only be modified...

Copy link
Contributor Author

@mohammadhunan-dev mohammadhunan-dev May 18, 2021

Choose a reason for hiding this comment

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

took this suggestion almost verbatim 👍

When using a ``forEach()`` loop or other :mdn:`iteration methods
<Web/JavaScript/Reference/Global_Objects/Set#iteration_methods>` to iterate
through a loop, {+service-short+} sets may be in a different order than
Copy link
Collaborator

Choose a reason for hiding this comment

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

through a loop,
to
through a set,

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Changed the sentence to: "When using a forEach() loop or other iteration method to traverse the set in a loop, the content of the Realm Set may be in a different order than originally written to."

originally written to. If you require an ordered version of your set, you
must implement that order yourself. You can do this by creating an array of
the set's values in the insertion order.
Copy link
Collaborator

Choose a reason for hiding this comment

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

The last sentence here is confusing. Are you saying the user should create an Array that gets updated every time the set is updated, and insert into the array in the order I want?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Essentially, that's it. I've altered it to be a bit clearer:

"You can do this by creating an array from the set, using Array.from(mySet), and updating that array when the set has been modified through a change listener."

Realm Object Models
-------------------
To define a property's values as a {+service-short+} set, specify the data type
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can this be shortened to:

To define a property type as a Set, specify the data type you want in the set, followed by <>

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That works, fixed now 👍

Create an Object With a Set
---------------------------
To create an object with a property that value is of the set data type, create a
Copy link
Collaborator

Choose a reason for hiding this comment

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

The wording here is confusing me.
Does this work?

To create an object that has one or more set properties, you must create the object within a write transaction. You then initialize the set-type properties as you would an array, by passing the initial values in brackets ([])

Copy link
Contributor Author

Choose a reason for hiding this comment

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

How about a combo of the two phrasings:

"To create an object with a {+service-short+} Set property, you must create the object within a write transaction. When defining your Realm object, initialize the set by passing an empty array or an array with your initial values."

Check if a Set has Specific Items
---------------------------------
To find out if a set has a particular value, pass the value to the ``set.has()`` method. The
Copy link
Collaborator

Choose a reason for hiding this comment

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

consider changing "To find out" to "To determine"

Copy link
Contributor Author

Choose a reason for hiding this comment

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

👍

Copy link
Contributor

@ccho-mongodb ccho-mongodb left a comment

Choose a reason for hiding this comment

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

There are some changes I think you should make and some nitpicks, but I'll approve since Caleb is also reviewing this.

--------
A {+service-short+} set is a special object that allows you to store a
Copy link
Contributor

Choose a reason for hiding this comment

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

I wonder if we should make it conspicuous that "Realm Set" is a term, either through capitalization similar to Javascript "Set" or using a bold format.
After reading the first couple of paragraphs, I'm also curious as to whether you can use Javascript Sets instead, or if I can only use Realm Sets since the title of the page is Sets.

Copy link
Contributor

Choose a reason for hiding this comment

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

I agree: A Realm Set is not a 1:1 mapping of a JavaScript Set (but heavily inspired by it).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fair point, I've altered it to be "Realm Set" consistently throughout the doc to indicate it is a term.

As for indicating that it is not a 1:1 mapping, I've included the line: "Realm sets work similarly to JavaScript Sets, except they must be a single type and can only be modified within a write-transaction"

source/sdk/node/data-types/sets.txt Outdated Show resolved Hide resolved
source/sdk/node/data-types/sets.txt Outdated Show resolved Hide resolved
source/sdk/node/data-types/sets.txt Outdated Show resolved Hide resolved
source/sdk/node/data-types/sets.txt Outdated Show resolved Hide resolved
source/sdk/node/data-types/sets.txt Outdated Show resolved Hide resolved
source/sdk/node/data-types/sets.txt Outdated Show resolved Hide resolved
source/sdk/node/data-types/sets.txt Outdated Show resolved Hide resolved
source/sdk/node/data-types/sets.txt Show resolved Hide resolved
examples/node/Examples/data-types.js Outdated Show resolved Hide resolved
@mohammadhunan-dev
Copy link
Contributor Author

mohammadhunan-dev commented May 18, 2021

@MongoCaleb per your top level comment: "Overall comment: I have no idea WHY I would use a set over an array. Perhaps JS developers already know that? Also, there is no section on how to retrieve a value from a set, and I assume that's because you can't -- you have to iterate? Is it worth any additional info about that?"

  • Added some info about the benefits of using a Set, but I'm opposed to directly mentioning Arrays in this doc since I think it may confuse readers. Fundamentally they are two different data types with unique use cases, see more about my reasoning for not directly mentioning Arrays here: (DOCSP-14573): set data type #1079 (comment)
  • About retrieving a value from a set, you can check if a set has a value (shown in Node.js > Sets # Check if a Set has Specific Items since sets are not retrievable via any index like arrays. Alternatively, you can iterate through the values in a set with a forEach loop, but I don't think we should show a snippet of that since iteration of a set using a forEach loop might confuse readers and imply that a set is similar to an array and since we don't guarantee traversal order.

source/sdk/node/data-types/sets.txt Outdated Show resolved Hide resolved
of your set, you must implement that ordering yourself.If you require an
ordered version of your set, you must implement that order yourself. You can
do this by creating an array from the set, using :mdn:`Array.from(mySet)
Copy link
Contributor

Choose a reason for hiding this comment

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

The spread operator (...) will also work.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed 👍

Mohammad Hunan Chughtai and others added 6 commits May 19, 2021 08:47
@mohammadhunan-dev mohammadhunan-dev merged commit c003974 into mongodb:new-data-types-node-rn May 19, 2021
mohammadhunan-dev pushed a commit that referenced this pull request May 19, 2021
* New data types node rn (#1040)

* new data types for node

* added data types to TOC

Co-authored-by: Mohammad Hunan Chughtai <mohammad.hunan@mongodb.com>

* fixed refs for node.js data types

* update doc

* Remove accidental changes

* added empty test file

* (DOCSP-15613): Added Node.js field types (#1041)

* Added node.js field types

* fixed wording

* fixed typo

* fix typo in mdn urls

* added additional data types

* fix monospace err

Co-authored-by: Mohammad Hunan Chughtai <mohammad.hunan@mongodb.com>

* (DOCSP-15613): Node.js collections type (#1044)

* Add content to Node.js > Data Types > Collections.txt

* Removed unneeded spacing

* clean up collections page + add headers

* Filled out collections page

* fix refs

* fix typo

* Update source/sdk/node/data-types/collections.txt

* Update source/sdk/node/data-types/collections.txt

* Update source/sdk/node/data-types/collections.txt

Co-authored-by: Dachary <dc@dacharycarey.com>

* fix rst

* fix woridng

* added ref

* removed unneeded word

* removed unneeded word

* added period

Co-authored-by: Mohammad Hunan Chughtai <mohammad.hunan@mongodb.com>
Co-authored-by: Dachary <dc@dacharycarey.com>

* (DOCSP-15613): Node.js embedded objects type (#1047)

* Added documentation of Node.js embedded objects under data types

* clean up relationships and embedded objects

* added CRUD examples for embedded obj

* added bluehawked examples

* fix rst syntax highlight + add description for deletes

* Update source/sdk/node/data-types/embedded-objects.txt

* Update source/sdk/node/data-types/embedded-objects.txt

* Update source/sdk/node/data-types/embedded-objects.txt

* Update source/sdk/node/data-types/embedded-objects.txt

* Update source/sdk/node/data-types/embedded-objects.txt

Co-authored-by: Mohammad Hunan Chughtai <mohammad.hunan@mongodb.com>

* Docsp 14569 mixed data type (#1064)

* attempt to add bluehawked mixed example snippets

* fixed wording

* Update source/examples/generated/node/data-types.codeblock.query-objects-with-mixed-values.js

* Update examples/node/Examples/data-types.js

Co-authored-by: Mohammad Hunan Chughtai <mohammad.hunan@mongodb.com>

* (DOCSP-14565): Dictionary data type (#1058)

* (DOCSP-14565): Dictionary Data Type - Node.js

* added unit tested + bluehawked dictionary examples

* Update examples/node/package.json

* removed unneeded file

* Added new examples for dictionaries

* update dictionary examples

* fix capitalization

* moved addlistener down

* fix comment

* update wording to be clearer on type usage in dict

* fix wording

* fix wording

Co-authored-by: Mohammad Hunan Chughtai <mohammad.hunan@mongodb.com>

* (DOCSP-14577): UUID (#1067)

* bump realmjs to 10.5.0-beta-1

* removed uuid as it's own page and added a subsection on it

* added uuid bluehawked snippet + readded uuid to toc

* added uuid examples

* removed uuid from field types as a paragraph

* update wording

* update wording

* fix passive voice

* add specific uuid example

* removed innacurate collections are homogenous (per mixed)

* (DOCSP-14573): set data type (#1079)

* added set examples + bluehawked

* literal included set exampkes

* added delete one set item example

* fix typo

* added descriptions to set

* fix grammar

* changed link + hunter to generic names

* added capitalization to clearly define Realm Set as a term

* fix wording + formatting"

* clarified wording

* fix literalincldue indentation

* fix typo

* more wording fixes

* clean up realm object model for set wording

* fix character names for examples

* clarified traversal order wording

* rst typo in <set>

* changed set to mySet

* fix overview wording

* updated overview to be more clear on differences between array

* updated create wording

* Update source/examples/generated/node/data-types.codeblock.remove-all-items-from-set.js

Co-authored-by: Kenneth Geisshirt <k@zigzak.net>

* Update source/examples/generated/node/data-types.codeblock.remove-specific-item-from-set.js

Co-authored-by: Kenneth Geisshirt <k@zigzak.net>

* Update source/sdk/node/data-types/sets.txt

Co-authored-by: Kenneth Geisshirt <k@zigzak.net>

* fix grammar + wording + changed Set to Realm.Set in js snippets

* Fix woridng

Co-authored-by: Kenneth Geisshirt <k@zigzak.net>

* fill out field types

* add react native data types as a copy of node data types

* add data types to toc

* fix rn mixed links

* attempt to add realm-js links

* more grammar and woridng fixes

* wording fixes

* Update source/sdk/node/data-types/uuid.txt

Co-authored-by: nate contino <nathan.contino@mongodb.com>

* change uuid note on rn to match node

* added note about obj id to both rn and node

* Update source/sdk/node/data-types/mixed.txt

Co-authored-by: nate contino <nathan.contino@mongodb.com>

* fix 'mixed' formatting on rn to match node

* change monospace to bold

* fix spacing errors

* fix wording

* correct supported types for mixed

* wording update

Co-authored-by: Mohammad Hunan Chughtai <mohammad.hunan@mongodb.com>
Co-authored-by: Dachary <dc@dacharycarey.com>
Co-authored-by: Kenneth Geisshirt <k@zigzak.net>
Co-authored-by: nate contino <nathan.contino@mongodb.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants