Skip to content

Add Tree Traversal in Emojicode. #471

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Nov 16, 2018
Merged

Add Tree Traversal in Emojicode. #471

merged 1 commit into from
Nov 16, 2018

Conversation

june128
Copy link
Member

@june128 june128 commented Oct 7, 2018

No description provided.

@june128 june128 added Implementation This provides an implementation for an algorithm. (Code and maybe md files are edited.) Hacktoberfest The label for all Hacktoberfest related things! labels Oct 7, 2018
thbwd
thbwd suggested changes Oct 8, 2018
View reviewed changes
@june128 june128 changed the title Add Tree Traversal in Emojicode. WIP Add Tree Traversal in Emojicode. Oct 8, 2018
@june128
Copy link
Member Author

june128 commented Oct 8, 2018

@thbwd Thank you very much for the reviews of the Emojicode PRs! 🙂👍

june128
june128 commented Oct 8, 2018
View reviewed changes
contents/tree_traversal/code/emojicode/tree_traversal.emojic Outdated
📗
❗️ 🍭 🍇
↪️ 🐔 children❗️ ▶️ 2 🍇
↩️↩️
Copy link
Member Author

Choose a reason for hiding this comment

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

@thbwd I have one more question tho. While looking through the documentation, I found that I can also return errors.
I think it would be more appropriate to return an error here, if there are more than two children.

How do I use an error type in this method? I think, the documentation doesn't describe, how to provide an error, when you have a method, that returns nothing.

Copy link

@thbwd thbwd Oct 9, 2018 •
edited
Loading

Choose a reason for hiding this comment

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

This is not possible by design. An error is either the expected value or an enum.

If you do not have a return type, there would not be an non-error type. Therefore we have either an enum indicating an error or no value at all, which is clearly what optionals are designed for.

This is also how the file package handles it:

https://www.emojicode.org/docs/packages/files/1f4c4.html#cm📻❗%EF%B8%8F

Copy link
Member Author

Choose a reason for hiding this comment

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

Ok, would the following be a correct implemetation?

Declare an enum to hold an error value:

🦃 ⏹ 🍇
  🔘 ⏫

  ❗️ 🔡 ➡️ 🔡 🍇
    ↪️ 🐕 🙌 🆕⏹⏫❗️ 🍇
      ↩️ 🔤The given tree is not binary!🔤
    🍉
    ↩️ 🔤🔤
  🍉
🍉

Then rewrite the Depth-First Search Recursive Inorder Binary method to return an optional holding the enum and adjust the returns according to that:

  ❗️ 🍭 ➡️ 🍬⏹ 🍇
    ↪️ 🐔 children❗️ ▶️ 2 🍇
      ↩️ 🆕⏹⏫❗️
    🍉

    ↪️ 🐔 children❗️ ▶️ 0 🍇
      🍭🐽 children 0❗️❗️
      😀 🔡 id 10❗️❗️
      🍭🐽 children 1❗️❗️
    🍉
    🙅 🍇
      😀 🔡 id 10❗️❗️
    🍉
    ↩️ 🤷‍♀️

and then call the method like this:

  ️↪️ 🍭tree❗️ ➡️ return 🍇
    😀 🔡return❗❗️️
  🍉

Copy link

@thbwd thbwd Oct 23, 2018 •
edited
Loading

Choose a reason for hiding this comment

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

Seems reasonable. Although — looking at this — a proper error handling mechanism for cases like this wouldn’t be a bad idea after all. Feel free to open an issue over at the Emojicode repo if you have ideas and time.

Copy link
Member Author

Choose a reason for hiding this comment

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

Alright, I used your suggested way of error handling for methods without a return value and opened an issue over at the emojicode repo: emojicode/emojicode#112 😀

@june128 june128 force-pushed the emojicode_tree_traversal branch 3 times, most recently from 0f9c86b to f244633 Compare October 27, 2018 00:20
@june128 june128 mentioned this pull request Nov 10, 2018
Closed
@june128
Copy link
Member Author

june128 commented Nov 15, 2018

@thbwd Are you happy with the code after the latest changes?

thbwd
thbwd approved these changes Nov 15, 2018
View reviewed changes
Copy link

@thbwd thbwd left a comment

Choose a reason for hiding this comment

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

Looks good! 👍 I hadn't received a notification about your changes, otherwise I would have approved this sooner, of course.

@june128 june128 force-pushed the emojicode_tree_traversal branch 4 times, most recently from 60a3fb0 to 0d36095 Compare November 16, 2018 00:46
@june128
Add Tree Traversal in Emojicode
50a8b94 
This is a combination of 5 commits.
This is the 1st commit message:

Add Tree Traversal in Emojicode

This is the 2nd commit message:

Remove unnecessary 🤜🤛.

This was suggested by thbwd:
"No need to wrap 🤜children_count🤛 into 🤜🤛actually."
(https://github.com/algorithm-archivists/algorithm-archive/pull/
471#discussion_r223291032)

This is the 3rd commit message:

Extract shared logic into a method.

This was suggested by thbwd:
"This is shared logic. I’d recommend you extract it into a method."
(https://github.com/algorithm-archivists/algorithm-archive/pull/
471#discussion_r223291140)

This is the 4th commit message:

Use documentation comments.

This was suggested by thbwd:
"Shouldn’t these be documentation comments? https://www.emojicode.org/
docs/reference/documentation.html"
(https://github.com/algorithm-archivists/algorithm-archive/pull/
471#discussion_r223291281)

This is the 5th commit message:

Add error as a potential return value for DFS RI Binary

To be able to return an error and to apply suggestion by thbwd
(https://github.com/algorithm-archivists/algorithm-archive/pull/
471#discussion_r223597592) the following changes were made:
- Declare an enum to hold the error value.
- Rewrite the Depth-First Search Recursive Inorder Binary method to
return an optional holding the enum and adjust the returns according
to that.
- Call the rewritten method and check, if the returned optional holds
error. If it does: Print out the error message defined in the enum.

Signed-off-by: Julian Schacher <jspp@posteo.net>
@june128 june128 force-pushed the emojicode_tree_traversal branch from 0d36095 to 50a8b94 Compare November 16, 2018 00:51
@june128 june128 changed the title WIP Add Tree Traversal in Emojicode. Add Tree Traversal in Emojicode. Nov 16, 2018
@june128
Copy link
Member Author

june128 commented Nov 16, 2018

@thbwd Awesome; thank you very much for the review!

Gathros
Gathros approved these changes Nov 16, 2018
View reviewed changes
Copy link
Contributor

@Gathros Gathros left a comment

Choose a reason for hiding this comment

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

Ill merge this for you.

@Gathros Gathros merged commit 50d3e36 into master Nov 16, 2018
@Gathros Gathros deleted the emojicode_tree_traversal branch November 16, 2018 21:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Butt4cak3 Butt4cak3 Butt4cak3 left review comments

@thbwd thbwd thbwd approved these changes

@Gathros Gathros Gathros approved these changes

Assignees
No one assigned
Labels
Hacktoberfest The label for all Hacktoberfest related things! Implementation This provides an implementation for an algorithm. (Code and maybe md files are edited.)
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@june128 @thbwd @Butt4cak3 @Gathros