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

add sample for text version of find #5368

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

RohitGupta200
Copy link

KT-35867 - Adds samples to Text find() function.

@SvyatoslavScherbina SvyatoslavScherbina added the Standard Library Sample Pull request with samples for stdlib API label Nov 28, 2024
@fzhinkin fzhinkin self-requested a review December 2, 2024 21:12
@fzhinkin fzhinkin self-assigned this Dec 2, 2024
*
* @sample samples.collections.Collections.Elements.find
*
* @sample samples.text.Strings.find
Copy link
Contributor

Choose a reason for hiding this comment

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

@RohitGupta200 please make sure that these generated files are updated via :tools:kotlin-stdlib-gen:run command.
See https://github.com/JetBrains/kotlin/blob/master/libraries/stdlib/ReadMe.md#code-generation for details.

specialFor(CharSequences) {
sample("samples.text.Strings.find")
}
specialFor(ArraysOfUnsigned) {
Copy link
Contributor

Choose a reason for hiding this comment

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

The original sample call should be left as is, without any additional specialFor predicates, and the call generating a sample specialized for char sequences should be placed right after it (wrapped into a predicate):

sample("samples.collections.Collections.Elements.find")
specialFor(CharSequences) {
    sample("samples.text.Strings.find")
}

@@ -435,6 +435,18 @@ class Strings {
assertPrints(matchDetails(inputString, toFind, 10), "Searching for 'ever' in 'Never ever give up' starting at position 10: Not found")
}

@Sample
fun find() {
Copy link
Contributor

Choose a reason for hiding this comment

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

This sample is not compilable, please run it locally before submitting the updated version.

@@ -435,6 +435,18 @@ class Strings {
assertPrints(matchDetails(inputString, toFind, 10), "Searching for 'ever' in 'Never ever give up' starting at position 10: Not found")
}

@Sample
fun find() {
val text = "k1o2t3l4i5n6"
Copy link
Contributor

Choose a reason for hiding this comment

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

Consider using a slightly more readable string, like "kotlin 2.1.0".
While it won't affect correctness, it would be much easier to understand which characters are digits and which are letters (distinguishing l from 1 might be tricky depending on a type font).

fun find() {
val text = "k1o2t3l4i5n6"

val firstNumberInText = text.find { it.isDigit() }
Copy link
Contributor

@fzhinkin fzhinkin Dec 2, 2024

Choose a reason for hiding this comment

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

Please inline an expression that uses find into assertPrints call (for example, like in the sample for last). A temporary variable doesn't add an additional context here and only distracts from grasping the semantics.

Copy link
Contributor

Choose a reason for hiding this comment

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

Well, it adds some context, but it might be better to add an explicit comment like // the index of the first digit, '1' and // the string does not contain uppercase letters.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Standard Library Sample Pull request with samples for stdlib API
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants