josh.smith
Request for code review - Weather Data kata
I’m relatively new to Elixir and have been tackling small programming challenges to become better acquainted with the language as I read books about it. Today during lunch I decided to try out the Weather Data “kata” exercise from Kata04: Data Munging - CodeKata
I got it working, though I don’t know if there is a better way. Here’s my code:
If someone with a lot of Elixir experience would please review my code and provide tips and/or suggestions, I’d appreciate it!
Thanks!
Marked As Solved
Qqwy
First and foremost: I find this a very nice and readable solution. Good job! ![]()
All right, here are my two cents:
- There does not seem to be a reason for using Enum.map and Enum.filter on lines 7 and 8 respectively. I think it is nicer to keep your Stream a Stream for as long as possible.
- Maybe your function
Weather.printmight rather be namedWeather.print_day_with_lowest_temperature_spread, although that would again be a very long name… In any case,printis a name that is not self-documenting. - Although documentation is not necessarily part of the Kata, because documentation is a first-class citizen in Elixir, I think it is a good practice to simply always document your public functions in one or two sentences.
- There exists the function
Enum.min_by/2, which you could use to clean up line 9 (Enum.reduce( &row_with_smaller_temperature_spread/2 )). That could become something likeEnum.min_by(&temperature_spread_of_row/1)defp temperature_spread_of_row([_, max, min), do: max - min - Nice job on outlining the functions in the pipeline. That is very readable
. - There is no check on columns that have the wrong data format. That is to say, you check if the first column in the row is a non-number (and thus treated as a non-day), but if one of the other columns cannot be parsed as an integer, the program will fail with a not very readable stack trace. (As it will fail when attempting to calculate the average temperature) You might want to add a check for this that raises a nicer exception.
- The pipeline on line 22 does not make the function more readable. Simply doing
Enum.map(row, fn x -> ... end)is probably better.
So, those were my nitpickings ^^'. In my opinion, you’ve done great on this exercise.
Also Liked
josh.smith
Thank you for taking the time to give me an in-depth review and such detailed feedback!
Popular in Discussions
Other popular topics
Categories:
Sub Categories:
Forums
Popular Tags
- #ecto
- #liveview
- #troubleshooting
- #learning-elixir
- #deployment
- #library
- #erlang
- #testing
- #genserver
- #mix
- #absinthe
- #remote-other
- #otp
- #plug
- #how-to-question
- #macros
- #postgres
- #channels
- #elixirconf
- #exunit
- #discussion
- #javascript
- #code-sync
- #podcasts
- #onsite
- #dialyzer
- #docker
- #authentication
- #umbrella
- #full-time-contract
- #podcasts-by-brainlid
- #ecto-query
- #elixir-ls
- #phoenix_html
- #iex
- #blog-post
- #graphql
- #genstage
- #ai
- #websockets
- #supervisor
- #advent-of-code
- #elixirconf-us
- #distillery
- #processes
- #forms
- #api
- #metaprogramming
- #security
- #performance








