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!

Last Post!

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

Crowdhailer
I’ve been hearing much about the new formatter and it’s something I have been keen to try. I find examples buy far the most illuminating...
248 19760 150
New
rower687
Hi all, I’ve been reading a lot about the “let it crash” term and how supervising processes and the whole messaging passing make an elixi...
New
New
chuck
Let me start by stating an assumption: Phoenix is a great approach to building REST APIs. There are many reasons for this, but I will ass...
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
AngeloChecked
What learn first? Rust or Elixir Hi Elixir community! I’m here because i want learn a new language. I’m a junior developer and mainly i ...
New
fireproofsocks
I’ve been working on an Elixir project that has required a lot of scripting. I usually reach for Elixir because I like it more (and in th...
New

Other popular topics Top

vonH
When I run the Plug and I recompile I wind up having to use Ctrl C to quit iex and start again. Witht the help of rlwrap I can use the cu...
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
AngeloChecked
What learn first? Rust or Elixir Hi Elixir community! I’m here because i want learn a new language. I’m a junior developer and mainly i ...
New
dblack
I’ve got an issue with an app and I’ve no idea of how to troubleshoot it. I’m hoping someone here might have seen something similar. I p...
New
Harrisonl
We have an ECS cluster with 4 services, where each task joins a single cluster, via discovery ECS discovery service. Currently when I de...
New
jason.o
In the code below, if the create action is not set to accept “extra_key” as an input, it errors out with a message shown above. Is there ...
New

We're in Beta

About us Mission Statement