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

Update Optional Comments In Kotlin Tutorials #4

Closed
AdamMc331 opened this issue Jun 12, 2017 · 10 comments
Closed

Update Optional Comments In Kotlin Tutorials #4

AdamMc331 opened this issue Jun 12, 2017 · 10 comments

Comments

@AdamMc331
Copy link
Contributor

https://chat.stackoverflow.com/transcript/message/37581345#37581345

@AdamMc331
Copy link
Contributor Author

Also comment out "..." lines.

@TimCastelijns
Copy link

In AddTaskActivity, change if (description?.text?.toString().isNullOrEmpty()) to use isNullOrBlank() instead, to also check if the text is not only made up of spaces

@TimCastelijns
Copy link

TimCastelijns commented Jun 12, 2017

In tutorial part 2, change val adapter = TaskAdapter() in onCreate to be class var var adapter: TaskAdapter? = TaskAdapter() as seen in the finished code.

Or perhaps initialize it as null, since the initial value of = TaskAdapter() is not used.

@TimCastelijns
Copy link

In part 3, maybe mention that string resources for AddTaskActivity xml are not yet defined (or just hardcode them)

@TimCastelijns
Copy link

In part 4, "We’ve already marked our tasks and serializable in part 2" should perhaps be "We’ve already marked our tasks as serializable in part 2" (and->as)

@TimCastelijns
Copy link

In part 4, adapter?.tasks = tasks is highlighted with "Smart cast to kotlin.collections.MutableList<com.package.Task>". Maybe add a note what that means

@AdamMc331
Copy link
Contributor Author

@TimCastelijns Regarding your comment about part 2, I think I should leave it as val adapter = TaskAdapter(), and then remove the setter inside of onCreate(). There's no need to do that, and there's also no need to make it optional. I will update the code this way.

AdamMc331 added a commit that referenced this issue Jun 12, 2017
@AdamMc331
Copy link
Contributor Author

@TimCastelijns Can you look over #5 and give me a thumbs up if you like it?

@TimCastelijns
Copy link

I upthumbed it. Not sure if you get a notification for that

@AdamMc331
Copy link
Contributor Author

I didn't, thanks! Will merge and update the site.

AdamMc331 added a commit that referenced this issue Jun 12, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants