1. 27
  1.  

  2. 3

    i love this little steps to bettter code

    1. 2

      I only disagree with two of these:

      • Using tagged assignments in with statements is great for figuring out where exactly something broke if the error tuples wouldn’t show it.
      • Using else in with statements to properly handle errors is reasonable to me, though I see where Chris is coming from in the case where he a) really embraces let it crash and doesn’t bother with more involved error checking code and b) believes that with statements should be rewritten into case statements if you care about error codes.

      Otherwise, excellent advice.

      1. 1

        I disagree with few of these:

        • Map.get/2 and Keyword.get/2 vs. Access - I prefer to just use Map and Keyword as not only it is IMHO cleaner to the reader about the expected types, it is also faster as there is no need for dynamic dispatch. Also Map.get/2 will work with structures that do not implement Access behaviour. If you want to have uniform access syntax, then you can have “frontend function” that does Map.new/1 on the options and pass the new map to other functions.
        • Don’t pipe into case statements - I like to do so, but only if that is the last thing in the function. That makes it much cleaner to my eyes and do not force me to create temporary variable somewhere at the top just to be ingested once.
        • Avoid else in with blocks - I agree with the fact that the tagged assignments are smell, but I completely disagree with the overall statement. else in with makes a lot of sense when you do not need to differ between each of the mismatches. So for example their own example that uses :fuse can be IMHO written much cleaner as:
          with :ok <- :fuse.check(:service),
               {:ok, result} <- call_service(id) do
            :ok = Cache.put(id, result)
            {:ok, result}
          else
            :blown -> Cache.fetch(id) # It is alternative to `Cache.get/1` that returns error-tuple instead
            {:error, _} = error ->
              :fuse.melt(:service)
              error
          end
          
          Yes, I fully agree, that irrecoverable errors should be exceptions. And if you are working with Plug (so it applies to Phoenix as well), then you can have :plug_status field with code that should be returned to user in case of error or you can implement Plug.Exception protocol to do the same thing.
        • Use for when checking collections in tests - instead, just write custom helper for that. You can also make it much better then by for example providing indices of the mismatched entries. It will also make test cleaner when you would do assert_items_match %Post{}, list instead.