josh.smith

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

Qqwy

TypeCheck Core Team

First and foremost: I find this a very nice and readable solution. Good job! :thumbsup:

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.print might rather be named Weather.print_day_with_lowest_temperature_spread, although that would again be a very long name… In any case, print is 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 like Enum.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 :slight_smile: .
  • 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

josh.smith

Thank you for taking the time to give me an in-depth review and such detailed feedback!

Where Next?

Popular in Discussions Top

matthias_toepp
I’d love to hear what people think about Wisp, the new Gleam web framework started by Gleam’s primary creator Louis Pilfold. Gleam, alon...
New
Jayshua
I recently came across the javascript library htmx. It reminded me a lot of liveview so I thought the community here might be interested....
New
thojanssens1
It would be nice to be able to define a redirect from one route to another from the router.ex file. E.g.: redirect "/", UserController, ...
New
sashaafm
Piggy backing a bit on @dvcrn topic BEAM optimization for functions with static return type?, I’ve been trying to understand in a deeper ...
New
AstonJ
If a newbie asked you about Phoenix Contexts, how would you explain the basics to them? Feel free to be as concise or in-depth as you li...
New
AstonJ
I’ve just started the Phoenix part of the utterly brilliant online course by @pragdave. On generating the Phoenix app he uses the --no-ec...
New
PragTob
Hey everyone, this has been on my mind for some time and I’d love your input on it! TLDR: I feel like maps are superioer for storing and...
New
eteeselink
Hi all, In the last days, two things happened: A blog post titled “They might never tell you it’s broken” made the rounds. It’s about ...
New
cblavier
Hey there, It’s been more than a year since we started using LiveView as our main UI library and building a whole library of UI componen...
New
sergio
Kind of like when jquery came out, it was super necessary. Existing drag and drop libraries have a bunch of baggage to support old browse...
New

Other popular topics Top

sorentwo
Hello! tl;dr Announcing Oban, an Ecto based job processing library with a focus on reliability and historical observability. After spen...
985 42920 311
New
albydarned
Hello all! I am typing this post from my new MacBook Pro with the M1 chip. I’m loving it so far, and will probably use it as my daily dr...
New
ovidiubadita
Hey all, I discovered Elixir and I love it. I always wanted to learn a functional programming and I intended to go for Haskell, but afte...
New
chrismccord
Phoenix 1.4.0 released Phoenix 1.4 is out! This release ships with exciting new features, most notably with HTTP2 support, improved deve...
688 30877 112
New
chrismccord
This release brings a number of exciting features, including integration with the new Phoenix LiveDashboard and Phoenix LiveView. There h...
New
alice
Hey, Just curious what are the main benefits of Elixir compared to Clojure? When is Elixir more useful than Clojure and vice versa? Th...
New
klo
Got a question about when to concat vs. prepending items to list then reversing to achieve appending. So i know lists boil down to [1 | ...
New
WestKeys
Currently suffering from paralysis by [HTTP client] analysis. This is rather unusual in Elixirland as there tends to be consensus on the ...
New
svb
Hi! Currently I want to submit a form by pressing the Enter key. However, since my input field is of type “textarea” this is just adds a...
New
vonH
In asking this question I am more interested about the expressiveness of the language itself and less concerned about the availability of...
New

We're in Beta

About us Mission Statement