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

Implement new functions in JsonArray #110

Merged
merged 2 commits into from
Jun 21, 2023
Merged

Implement new functions in JsonArray #110

merged 2 commits into from
Jun 21, 2023

Conversation

7hong13
Copy link
Contributor

@7hong13 7hong13 commented Jun 21, 2023

What this PR does / why we need it?

Some public functions of JsonArray exist in JS and iOS SDK, but not in the Android SDK. These functions include:

  1. insertAfter(), insertBefore()
  2. moveAfter(), moveBefore(), moveFirst(), moveLast()
  3. splice()

Regarding the insert functions (1), it is kind of tricky to implement them as separate functions in the Android SDK, so I implemented the same feature of insertAfter() by modifying the put methods. I guess insertAfter() can cover the expected behaviors of insertBefore(), so I did not implement it.

The move functions (2) are implemented the same way as in other SDKs.

As for splice(), it is a JavaScript method and seems unnecessary for Kotlin. (I am considering adding replace() instead of splice(), but maybe it would be better to keep it until it is actually necessary).

+) All the JsonElements have id, so I defined id property in JsonElement instead of implementing it in each of its child class.

Any background context you want to provide?

What are the relevant tickets?

Fixes #

Checklist

  • Added relevant tests or not required
  • Didn't break anything

@7hong13 7hong13 added the enhancement 🌟 New feature or request label Jun 21, 2023
@7hong13 7hong13 self-assigned this Jun 21, 2023
@7hong13 7hong13 changed the title Implement new functions in CrdtArray Implement new functions in JsonArray Jun 21, 2023
@codecov
Copy link

codecov bot commented Jun 21, 2023

Codecov Report

Merging #110 (a748e68) into main (b5cf896) will increase coverage by 1.56%.
The diff coverage is 88.57%.

@@             Coverage Diff              @@
##               main     #110      +/-   ##
============================================
+ Coverage     78.54%   80.11%   +1.56%     
- Complexity      545      582      +37     
============================================
  Files            53       53              
  Lines          2620     2640      +20     
  Branches        357      357              
============================================
+ Hits           2058     2115      +57     
+ Misses          368      332      -36     
+ Partials        194      193       -1     
Impacted Files Coverage Δ
...ain/kotlin/dev/yorkie/document/json/JsonCounter.kt 93.75% <ø> (+5.51%) ⬆️
...main/kotlin/dev/yorkie/document/json/JsonObject.kt 90.54% <ø> (+1.20%) ⬆️
...c/main/kotlin/dev/yorkie/document/json/JsonText.kt 85.18% <ø> (+1.54%) ⬆️
...ain/kotlin/dev/yorkie/document/json/JsonElement.kt 75.00% <33.33%> (-13.89%) ⬇️
.../main/kotlin/dev/yorkie/document/json/JsonArray.kt 95.65% <93.75%> (+42.22%) ⬆️

... and 5 files with indirect coverage changes

@7hong13 7hong13 merged commit 2983e04 into main Jun 21, 2023
7 checks passed
@7hong13 7hong13 deleted the json_array branch June 21, 2023 13:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement 🌟 New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants