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

Clean up on the List and List-Method Isles #2404

Merged
merged 19 commits into from
Apr 27, 2021

Conversation

BethanyG
Copy link
Member

This still needs nits and spelling patrol, but I am hoping that the concept related writing is now in good shape.

Re-worked the following:

  • about.md for lists and list-methods
  • indtroduction.md for lists and list-methods
  • introduction.md for chaitanas-colossal-coaster (covering list-methods)
  • indroduction.md for elyses-enchantments (covering lists)

Copy link
Member

@IsaacG IsaacG left a comment

Choose a reason for hiding this comment

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

Review, part 1 (file 1)

concepts/list-methods/about.md Outdated Show resolved Hide resolved
concepts/list-methods/about.md Outdated Show resolved Hide resolved
concepts/list-methods/about.md Outdated Show resolved Hide resolved
concepts/list-methods/about.md Outdated Show resolved Hide resolved
concepts/list-methods/about.md Outdated Show resolved Hide resolved
concepts/list-methods/about.md Outdated Show resolved Hide resolved
concepts/list-methods/about.md Outdated Show resolved Hide resolved
concepts/list-methods/about.md Outdated Show resolved Hide resolved
concepts/list-methods/about.md Outdated Show resolved Hide resolved
Copy link
Member

@IsaacG IsaacG left a comment

Choose a reason for hiding this comment

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

Part 2 of 2

concepts/list-methods/about.md Outdated Show resolved Hide resolved
3. ticket_type is an `int` with 1 for the express queue and 0 for the normal queue.
4. person_name is the name of the person to be added to the respective queue.

1. ticket_type is an `int` with 1 == express_queue and 0 == normal_queue.
Copy link
Member

Choose a reason for hiding this comment

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

This looks too much like a Yoda Conditional and like you're comparing an int to a list.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yup. Agree. Needs ... love. Do you think we should chuck and re-write this exercise? Its fine if you do. I was resisting, since there is a lot here..but the queue thing is starting to not work for me. Thoughts?

Copy link
Member

Choose a reason for hiding this comment

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

The multi-queue is only used for this one function so it can probably be dropped without impacting the other functions. If you want the add() to do more than append() you can have it only add if the person is not in the queue already.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants