Skip to content

Morse decoder with Elixir#3

Merged
matiasbeckerle merged 2 commits intoMakingSense:masterfrom
matiasbeckerle:morse-elixir
Mar 17, 2018
Merged

Morse decoder with Elixir#3
matiasbeckerle merged 2 commits intoMakingSense:masterfrom
matiasbeckerle:morse-elixir

Conversation

@matiasbeckerle
Copy link
Contributor

@matiasbeckerle matiasbeckerle commented Jul 16, 2017

Probably not the best approach, but it works. I did a hack which I will try to improve some day. Added the whole project so anyone can install Elixir and run the project easily. Anyways, the important files:

  • lib/morse.ex.
  • test/morse_test.exs

Here is the sausage: https://github.com/MakingSense/coding-dojo/pull/3/files#diff-034ed42181ecd30c24e51db7f0c06898R18

2017-07-16_1701

The hack explanation:

I wanted to solve the whole thing using the pipe operator |>. The problem I had was that after splitting by words I also need to split by letters. But at some point there is a return that it was sending me the words and not the letters. This is of course the lack of knowledge over the language (this is my first attempt to code something in this language). The hacks consists on translate the words separator (triple space) as another morse symbol.

@matiasbeckerle
Copy link
Contributor Author

Your 👀 are appreciated here too @JasonSoares (my first code attempt with Elixir).

@DanielAltamirano
Copy link
Contributor

Awesome! I get to learn elixir as well! Thanks! I'll review it later! 👍

Copy link
Member

@AlphaGit AlphaGit left a comment

Choose a reason for hiding this comment

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

I think it's a very good attempt! Out of curiosity, how long did it take you to get this done?


Documentation can be generated with [ExDoc](https://github.com/elixir-lang/ex_doc)
and published on [HexDocs](https://hexdocs.pm). Once published, the docs can
be found at [https://hexdocs.pm/morse](https://hexdocs.pm/morse).
Copy link
Member

Choose a reason for hiding this comment

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

Aw man, you DID publish them: https://hexdocs.pm/morse/Morse.html#content

That's so cool!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Lol. That wasn't me 😄

But I definitively should clean up these docs. I'm probably to move this code to a personal repo and improve it.

@matiasbeckerle
Copy link
Contributor Author

matiasbeckerle commented Jul 18, 2017

Thanks @DanielAltamirano and @AlphaGit. It was really interesting, and I liked the challenge.

Out of curiosity, how long did it take you to get this done?

Well I didn't measure it, because it is a new language for me. But I think I was between 3 and 4 hours which involves:

  • Environment installation.
  • Read & learn the basics.
  • Learn the basics of mix tool.
  • Setup the project.
  • Do the solution.
  • Spent at least 2 hours trying to improve that solution because I wanted something slightly different. Without success.

@AlphaGit
Copy link
Member

@matiasbeckerle Gotcha -- thanks!

@matiasbeckerle
Copy link
Contributor Author

Hey folks... should I close this PR? I'm cleaning up all my open PRs and I've found this one. @DanielAltamirano @AlphaGit

@AlphaGit
Copy link
Member

@matiasbeckerle I'm fine with that!

@andresmoschini
Copy link
Member

Hi @matiasbeckerle, I did not have time to review this yet (and neither fix mine one), but I think that you should merge it, it will be a shame if you close it without merge.

@matiasbeckerle
Copy link
Contributor Author

Thank you @AlphaGit @andresmoschini. I will wait for @DanielAltamirano's blessing before moving forward.

@andresmoschini
Copy link
Member

andresmoschini commented Feb 23, 2018 via email

@mbeckerle
Copy link

Note: @andresmoschini - mbeckerle (me) is not same as @matiasbeckerle. You wanted to comment for him.

@andresmoschini
Copy link
Member

Ooops, sorry Mike, and thanks for your nice and polite message to make me know about my error.

@matiasbeckerle
Copy link
Contributor Author

Ok based on @andresmoschini and @AlphaGit comments I will merge this one. We can revert it if needed.

@matiasbeckerle matiasbeckerle merged commit 3bb899a into MakingSense:master Mar 17, 2018
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

Successfully merging this pull request may close these issues.

5 participants