- Notifications
You must be signed in to change notification settings - Fork 10.6k
[stdlib] Revising Array Internal Comments and Example Code for Consistency. #70894
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
Conversation
…tency In the documentation from line 92 to 96, there is a statement that reads, "Using a negative number or an index equal to or greater than count triggers a runtime error." I believe that a more accurate example is needed for this statement. Accordingly, I have modified the existing example code so that the example code and the explanatory comments are consistent. With these modifications, users reading the comments can understand more clearly, and it is anticipated that they won't have to go through the process of looking up additional documentation. Previous code: ```swift /// You can access individual array elements through a subscript. The first /// element of a nonempty array is always at index zero. You can subscript an /// array with any integer from zero up to, but not including, the count of /// the array. Using a negative number or an index equal to or greater than /// `count` triggers a runtime error. For example: /// /// print(oddNumbers[0], oddNumbers[3], separator: ", ") /// // Prints "1, 7" /// /// print(emptyDoubles[0]) /// // Triggers runtime error: Index out of range ``` Modified code: ```swift /// You can access individual array elements through a subscript. The first /// element of a nonempty array is always at index zero. You can subscript an /// array with any integer from zero up to, but not including, the count of /// the array. Using a negative number or an index equal to or greater than /// `count` triggers a runtime error. For example: /// /// print(oddNumbers[0], oddNumbers[3], separator: ", ") /// // Prints "1, 7" /// /// print(oddNumbers[-1]) /// // Using a negative number /// // Triggers runtime error: Index out of range /// /// print(oddNumbers[8]) /// // Equal to `count` /// // Triggers runtime error: Index out of range /// /// print(oddNumbers[9]) /// // Greater than `count` /// // Triggers runtime error: Index out of range /// /// print(emptyDoubles[0]) /// // Triggers runtime error: Index out of range ```
| Whenever you have time, I would appreciate it if you could provide a review. Thank you. |
| Could it be that the review is not proceeding due to a mistake on my part? If there is anything wrong, please let me know, and I will make the necessary corrections. cc: @glessard |
@devKobe24: You opened the request on a three-day weekend in the US (Martin Luther King Jr Day), so many of us haven’t been back to work since then. I’ll have a look in the morning. |
invalidname left a comment
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.
The paragraph above describes these cases as throwing a runtime error, but it doesn't hurt to spell it out explicitly like this. Maybe this helps the new Swift developer who hasn't been using arrays in various languages for many years.
amartini51 left a comment
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.
Thanks for this suggested change! I think this approach has some issues, and described an alternate approach in comments.
In accordance with your requested changes, I have respectfully removed the comments on lines 101 to 112.
| In accordance with your requested changes, I have respectfully removed the comments on lines 101 to 112. |
| Have I made any mistakes in my actions or comments throughout the process that have delayed the review? cc: @glessard |
Thanks. With that change, this PR no longer makes any changes. Would you like to try turning the prose into a bulleted list, as discussed in the comments? I'll try to make a PR next week with that change, if time permits. I think the history might be cleaner if we close this PR without merging, and open a new one to try doing this a different way. This branch picked up several merge commits that aren't needed. |
The comments from line 92 to 96 mention that using an index that is negative or greater than the count will cause a runtime error. I believe we need a more precise example for this comment. Accordingly, I have made edits to ensure that the example code and the explanatory comments are consistent, using bullet points for clarity and coherence. This should allow readers to understand the content more clearly and consistently, reducing the need to search for additional documentation. Previous Code: ```swift /// You can access individual array elements through a subscript. The first /// element of a nonempty array is always at index zero. You can subscript an /// array with any integer from zero up to, but not including, the count of /// the array. Using a negative number or an index equal to or greater than /// `count` triggers a runtime error. For example: /// /// print(oddNumbers[0], oddNumbers[3], separator: ", ") /// // Prints "1, 7" /// /// print(emptyDoubles[0]) /// // Triggers runtime error: Index out of range ``` Modified Code: ```swift /// You can access individual array elements through a subscript. The first /// element of a nonempty array is always at index zero. You can subscript an /// array with any integer from zero up to, but not including, the count of /// the array. Using a negative number or an index equal to or greater than /// `count` triggers a runtime error. For example: /// /// print(oddNumbers[0], oddNumbers[3], separator: ", ") /// // Prints "1, 7" /// /// print(emptyDoubles[0]) /// // Triggers runtime error: Index out of range /// /// - print(oddNumbers[-1]) /// - Using a negative number /// - Triggers runtime error: Index out of range /// /// - print(oddNumbers[8]) /// - Equal to `count` /// - Triggers runtime error: Index out of range /// /// - print(oddNumbers[9]) /// - Greater than `count` /// - Triggers runtime error: Index out of range ```
Thank you for your kind response. |
| Here's an example of using a bulleted list: main...amartini51:swift:array_traps It still merits discussion whether this is actually an improvement. |
Allow me to share my personal opinion: I understand Swift to be a user-friendly language, a sentiment I have experienced firsthand. Consequently, even for someone new to Swift, the 'Jump to definition' feature provides clear, user-friendly examples that facilitate immediate comprehension through direct engagement. Additionally, this reduces the need to search for supplementary documentation, swiftly bringing users to appreciate another of Swift's strengths: its flexibility. Lastly, reading comments can open up a vista for exploring deeper knowledge. I, too, have often stumbled upon other texts while reading well-explained, user-friendly examples in comments, leading to the acquisition of new knowledge. For these reasons, I believe there are more positive aspects from a user's perspective. |
In the documentation from line 92 to 96, there is a statement that reads, "Using a negative number or an index equal to or greater than count triggers a runtime error."
I believe that a more accurate example is needed for this statement.
Accordingly, I have modified the existing example code so that the example code and the explanatory comments are consistent.
With these modifications, users reading the comments can understand more clearly, and it is anticipated that they won't have to go through the process of looking up additional documentation.
Previous code:
Modified code: