-
Notifications
You must be signed in to change notification settings - Fork 3.8k
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
offsetbycodepoints function added #1590 #2453
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@sumitdethe27 I did a first review and left a few comments.
content/java/concepts/strings/terms/offsetByCodePoints/offsetByCodePoints.md
Outdated
Show resolved
Hide resolved
content/java/concepts/strings/terms/offsetByCodePoints/offsetByCodePoints.md
Outdated
Show resolved
Hide resolved
content/java/concepts/strings/terms/offsetByCodePoints/offsetByCodePoints.md
Outdated
Show resolved
Hide resolved
content/java/concepts/strings/terms/offsetByCodePoints/offsetByCodePoints.md
Outdated
Show resolved
Hide resolved
content/java/concepts/strings/terms/offsetByCodePoints/offsetByCodePoints.md
Show resolved
Hide resolved
…yCodePoints.md Co-authored-by: SSwiniarski <[email protected]>
…nto offsetMethod
@SSwiniarski I have made the changes, Please review. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@sumitdethe27 More comments.
``` | ||
|
||
- `startIndex` (int): The starting index in the string from which the offset is applied. | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remove extra line break.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OKay
New Index: 7 | ||
``` | ||
|
||
## Example 2 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You haven't addressed my issue with the duplicative examples:
The next two examples are essentially the same as the first one. It might be good to show an example of fetching a character from the middle of a string of completely non-Latin script.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@SSwiniarski In Example 3, I have added a completely non-latin script.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@sumitdethe27 some more comments.
|
||
## Example 3 | ||
|
||
In this example, the string str contains the text `"Hello, 世界!".` We specify the starting index as 0 and the code point offset as 5. The `**offsetByCodePoints()**` method returns the new index after applying the offset, which is 5 in this case. It means that the character at index 5 is the desired character. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You haven't updated the description to match.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for catching that mistake. I apologize for the confusion caused by the incorrect description. I have now updated the code and description to reflect the correct behavior.
New Index: 7 | ||
``` | ||
|
||
## Example 2 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you remove example two, as it pretty much duplicates example one.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sure
@SSwiniarski please review the changes. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@sumitdethe27 I forwarding this off to a second review.
Thankyou |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@sumitdethe27 I've added several comments. Please update the entry, and tag me when you've finished making changes. Thanks
content/java/concepts/strings/terms/offsetByCodePoints/offsetByCodePoints.md
Outdated
Show resolved
Hide resolved
content/java/concepts/strings/terms/offsetByCodePoints/offsetByCodePoints.md
Outdated
Show resolved
Hide resolved
content/java/concepts/strings/terms/offsetByCodePoints/offsetByCodePoints.md
Outdated
Show resolved
Hide resolved
content/java/concepts/strings/terms/offsetByCodePoints/offsetByCodePoints.md
Outdated
Show resolved
Hide resolved
content/java/concepts/strings/terms/offsetByCodePoints/offsetByCodePoints.md
Outdated
Show resolved
Hide resolved
content/java/concepts/strings/terms/offsetByCodePoints/offsetByCodePoints.md
Outdated
Show resolved
Hide resolved
content/java/concepts/strings/terms/offsetByCodePoints/offsetByCodePoints.md
Outdated
Show resolved
Hide resolved
…yCodePoints.md Co-authored-by: caupolicandiaz <[email protected]>
…yCodePoints.md Co-authored-by: caupolicandiaz <[email protected]>
…yCodePoints.md Co-authored-by: caupolicandiaz <[email protected]>
…yCodePoints.md Co-authored-by: caupolicandiaz <[email protected]>
…yCodePoints.md Co-authored-by: caupolicandiaz <[email protected]>
…yCodePoints.md Co-authored-by: caupolicandiaz <[email protected]>
…yCodePoints.md Co-authored-by: caupolicandiaz <[email protected]>
@caupolicandiaz I have made the changes |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@sumitdethe27 @SSwiniarski Looks good to merge
👋 @sumitdethe27 |
@sumitdethe27 Congrats on the new entry. Live link here: https://www.codecademy.com/resources/docs/java/strings/offsetByCodePoints You're currently anonymous. If you don't wish to be anonymous, check out this forum thread on how to link your GitHub and Codecademy profiles: https://discuss.codecademy.com/t/my-contribution-is-live-but-im-not-being-shown-as-a-contributor-why-is-this/758036 Note: it may take a few minutes for the change to register on the entry page. |
Thankyou @SSwiniarski |
Java Strings: .offsetByCodePoints() Method Added- Issue fixed #1590
Type of Change
Checklist
main
branch.