How can I do a better code and architecture design with Elixir Erlang and FP principles

Chat / Discussions
,
1

Hi, I’ve been developing a project for a few months and since the beginning i’ve been paying attention to the design of my code, writing it modular, well separated and those kind of things, but it’s been growing and i’m missing something I dont really know what. I’m often kind of lost of what a module or service returns, you now like when a module calls a module that calls a module that calls another module, and I caught myself open a few archives to check the returns. I think what is missing for me it’s some kind of architecture guide, something like interfaces to know exactly what x or y returns, so I’m here asking for some general or personal resources into that.
To give an example, I have the a controller that at some point calls another module

defp handle_answer(%{"payload" => payload, "status" => status, "meta" => meta}) when is_boolean(status) do
    case status do
      true -> handle_paid_pix(payload)
      false -> handle_failure_pix(payload, meta)
    end
  end

  defp handle_answer(_), do: {:error, FE.initerror("validation", "invalid payload")}

  defp handle_paid_pix(payload), do: TransactionRealm.handle_pix_paid(payload)
  defp handle_failure_pix(payload, meta), do: TransactionRealm.handle_pix_failed(payload, meta)

but then in this module I got more calls to another modules

 def handle_pix_paid(payload) do
    PixTransaction.handle_pix_paid(payload)
  end

  def handle_pix_failed(payload, meta) do
    PixTransaction.handle_failure_pix(payload, meta)
  end


    def handle_pix_paid(payload) do
        with {:ok, _} <- MnesiaPendingContractsDomain.update_payment_status({:payload,  payload, :pix_paid}),
            {:ok, _} <- EctoContractsDomain.update_payment_status({:payload, payload, :pix_paid}) do
            :ok
        else
            {:error, _} = error -> error
        end
    end

    def handle_failure_pix(payload, meta) do
        with {:ok, _} <- MnesiaPendingContractsDomain.update_contract_meta({:payload, payload, meta}) do
            :ok
        else
            {:error, _} = error -> error
        end
    end

I dont know if its on my mind but really feels like a terrible spagetthi code terrible typed and error prone. As I said at the beginnig designing that way was no problem, but i’m really a constraint that ties my code which makes it cohesive and well-fitted. Sorry if it’s a very abstract question, I tried to explain it as best I could.

2

Perhaps this is a silly reply but have you tried just not doing that? It’s not clear to me if these examples are your real code or not, or why you’ve chosen to structure your code in this way, but it looks like a lot of those functions aren’t actually doing anything, at all.

This is why I typespec all of my functions :slight_smile:

4 Likes
3

Definitely very abstract and I don’t know any resources off hand. My best advice would be to “unread” Clean Code :sweat_smile:

But seriously, I can point out a few things here.

As an easy one, there is little point in extract single-user private function calls (as in the case status do line). This doesn’t help readability at all, it just makes it so you have to keep jumping around to get the full picture.

This sorta continues on into the bit with handle_pix_paid. Seems that is abstracting over the complicated arguments required for Mnesia and Ecto ({:payload, payload, :pix_paid}) but you could just push that complexity into their respective modules and called them directly, something like MnesiaPendingCtractsDomain.complete_payment(payload). Just by the names too it seems like your modules may be way too granular. Without seeing the implementation, my intuition is that you are mimicking the OO service pattern. The best FP advice I can give you is to just start with simple functions. Start organizing as you need to. Again, since modules don’t hold state, they are much easier to refactor than classes since you can just move a function to another module with realative ease.

3 Likes
4

Not silly at all :slight_smile:

1 Like
5

Hm I get what you’re saying, I choose to design that way not to import too much of clean code but just to be able to see where things are done (transactions related on the transactions module, pix related on the pix module) but I indeed made it too granular, I probably can chain more things together, like make Pix a submodule of Transactions, keeping separation but mantaining both on the same place. Also the abstracting complexity into the their modules is a nice tip, thanks :slight_smile:

1 Like
6

This doesn’t answer your more abstract questions, but sometimes I find a combination of lower-level ideas can help. That code above is useless. That’s the default behavior of with: any clause that doesn’t pattern match is then offered to the clauses in else and if there is still no match, then the non-matching value is returned unchanged.

7

really good point, thank you :slight_smile: