Code Critique: struggling with 'correct' functional style

Questions / Help
1

Hello everyone

I’m looking for some feedback on this code I wrote: redirect-checker (I didn’t paste the code here as it’s about 40 lines).

Thanks!

2

Welcome @manckaert!

Looks good to me. My main metric is whether I can read & understand it quickly, and I could.

mix format is your friend (although, like all friends, I have some disagreements from time to time). It’s good to get into the habit of using it - the community’s eyes are used to running over code formatted in a particular way. There’s only one block of code that got reorganised when I ran it:

    redirect_to =
      case location_header do
        {"Location", loc} -> loc
        nil -> nil
      end

If this becomes part of a bigger system, you may find passing tuples around gets confusing, in which case upgrade to a map or (better still) a struct.

Also, HTTPoison.get would, I imagine, be quite slow with lots of wait time. If you want to learn a bit more about the power of BEAM, you could have a crack at parallelising. I’m sure there are many articles explaining how - my google-fu turned up a reasonable one as a starting point: Write Unbelievably Clean Concurrent Code in Elixir Using the Task Module | TopTechSkills.com

4 Likes
3

Thank you @mindok for the welcome and taking a look!

You’re right about the formatting, I come from Python mostly and there black is a lifesaver - especially in a team. I was mostly relying on how Emacs formatted my code but I fixed and issue with elixir-ls and now the section you highlighted gets formatted correctly.

I had already started rewriting it using Tasks as I had already noticed the IO is killing performance when doing the HTTP requests. It’s turning out quite nice but currently the “result collection” is pretty messy :sweat_smile:

Thank you again for your time!

1 Like
4

have you tried dbg instead of inspect?

5

I did now :slight_smile: Thanks for pointing that out. I had heard / read about dbg but kept using IO.inspect out of pure habbit. It’s quite useful to see the code line where you called dbg if you have a couple of ‘debug’ statements in your code.

6

Welcome @manckaert!

Additional to the comments written above, you could split the last function declaration:

# matches `foo,bar,foo` as the third argument should be the same as the first.  
# The `to_url` can be seen as an alias and makes the code easier to understand.
def verify_response({redirect_to, from_url, to_url = redirect_to}), do:
      %{result: :match, from_url: from_url, to_url: to_url}

# matches `foo,bar,baz`
def verify_response({redirect_to, from_url, to_url}), do:
      %{result: :no_match, from_url: from_url, to_url: to_url, redirect_to: redirect_to}

In your code, the function is quite small so it’s easy to digest. This is just to show you a technique you might not know already :slight_smile:

2 Likes
7

Thanks @BartOtten! I had thought of doing the verification like that as it feels more “elixir - like” but never got it working. Thanks for showing me the correct way :smiley:

1 Like