-
Notifications
You must be signed in to change notification settings - Fork 4.2k
update: improve Classes page #4786
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
base: master
Are you sure you want to change the base?
Conversation
b4d1916
to
85c4e7f
Compare
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.
My main question is: what are the things we want the reader to know, after they've finished reading the updated "Classes" page? Is it basic syntax for classes, details about constructors, details about initialization, smth else? Right now the section is a bit all over the place, I feel.
In other words, when we set to improve the page, what did we want to achieve?
82db250
to
9fb5f07
Compare
3ce95e1
to
3871cf7
Compare
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 the updates! In general, I like the new version.
I narrowed down my main concern to the struggle between "being succinct and to the point targeting existing developers" in the old version vs "being very detailed and explaining very basic things targeting novice developers" in the new one. I don't know how to find a good balance here besides taking a look at the actual feedback of users and what they are missing.
Additionally, I think it's important to remember that, while all and every feedback is important, a lot of people who were completely satisfied with the current succinct and to-the-point version didn't leave their positive feedback. We might "over-optimize" for the smaller part of our target audience which is more vocal and more novice, and they want very basic things explained in detail.
Note: feel free to disregard my general feedback and only incorporate the technical notes, as you are the person who definitely knows more about how to write good technical documentation. If you believe it should be done in a specific way, then that's what we should do.
This PR is stale because it has been open 30 days with no activity. Remove stale label or comment or this will be closed in 7 days. |
This PR is stale because it has been open 30 days with no activity. Remove stale label or comment or this will be closed in 7 days. |
3871cf7
to
e809a7e
Compare
Thanks for your detailed review! |
80f3e37
to
aa0edf2
Compare
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 the update, it looks much more focused and to the point, compared to the previous version! I've left some comments, but overall I don't have a lot to add.
In all the places where we are technically not entirely correct, we are ambiguous enough and pragmatic enough; to fix these places, we would need to introduce too many additional things and details, which is not really appropriate for this page, I feel.
132ed5c
to
e3e1830
Compare
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.
:lgtm:
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.
Huge effort Ale! 🚀 It's definitely looking much better than the original. Thanks for the hard work!
Please take a look at my suggestions.
There's some for improving names in code samples as well as adding code comments for printed statements within the main() function of samples.
I made a couple minor structural suggestions and asked a couple of questions for clarification.
When you [create an instance of a class](#creating-instances-of-classes), you are creating | ||
a concrete object based on that blueprint. | ||
|
||
Kotlin offers a concise syntax for declaring classes. To declare a class, use the `class` keyword |
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.
Kotlin offers a concise syntax for declaring classes. To declare a class, use the `class` keyword | |
Kotlin offers concise syntax for declaring classes. To declare a class, use the `class` keyword |
* [Nested and inner classes](nested-classes.md) | ||
* [Object declarations](object-declarations.md) | ||
|
||
Both the class header and class body can be minimal. |
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.
What do you mean by "minimal" here?
* [Object declarations](object-declarations.md) | ||
|
||
Both the class header and class body can be minimal. | ||
If the class does not have a body, you can omit the curly braces `{}`: |
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.
If the class does not have a body, you can omit the curly braces `{}`: | |
If the class doesn't have a body, you can omit the curly braces `{}`: |
is declared in the class header, and it goes after the class name and optional type parameters. | ||
```kotlin | ||
// Person class with a primary constructor | ||
// that nitializes the name property |
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.
// that nitializes the name property | |
// that initializes the name property |
``` | ||
|
||
## Constructors | ||
Here's an example of a class with a header, body, and an [instance created](#creating-instances-of-classes) from it: |
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.
Here's an example of a class with a header, body, and an [instance created](#creating-instances-of-classes) from it: | |
Here's an example that declares a class with a header and body, then [creates an instance](#creating-instances-of-classes) from it: |
This doesn't sound natural to me. I've suggested an alternative. I think it's because the "class with a header and body" is the important part. It separates it from the instance creation which is a different level of abstraction.
} | ||
``` | ||
{kotlin-runnable="true"} | ||
|
||
If a non-abstract class does not declare any constructors (primary or secondary), it will have a generated primary constructor | ||
with no arguments. The visibility of the constructor will be public. | ||
If there's a class that does not declare any constructors (primary or secondary), it will have an implicit primary constructor |
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.
I think it would be good to put the rest of this content under a separate header. It doesn't really fit under "Secondary constructors". Maybe "Without constructors"?
|
||
fun main() { | ||
// Creates an instance of the Person class | ||
Person("John", 30) |
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.
Person("John", 30) | |
Person("John", 30) | |
// Person created: John, age 30. |
// and converts it to an integer | ||
constructor(name: String, age: String) : this(name, age.toIntOrNull() ?: 0) { | ||
println("$name created with converted age: $age") | ||
// Bob created with converted age: 8 |
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.
// Bob created with converted age: 8 |
|
||
fun main() { | ||
// Uses the secondary constructor with a String age | ||
Person("Bob", "8") |
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.
Person("Bob", "8") | |
Person("Bob", "8") | |
// Bob created with converted age: 8 |
I think it makes more sense to add the printed statement here for the constructor.
// to the primary constructor | ||
constructor(name: String) : this(name, 0) { | ||
println("Person created with default age: $age and name: $name.") | ||
// Person created with default age: 0 and name: Alice. |
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 move the code comments to where they will appear when triggered by the main function?
This PR revamps the content and code examples in the Classes page following external feedback.
Some of the key changes are:
Improve the whole page and revamp the code snippets.
Explain the use case for multiple init blocks.
Explain constructor chaining.
Add examples for primary constructors.
Clarify the open keyword.
Explain how to access a class property from a class member function.
Use the term "object" consistently.
Clarify what abstract classes are.
Provide example companion objects.