Am I doing this right? Pattern matching in controller methods

In my first Phoenix app, I have an action which serves an XML phonebook to a certain SIP handset. The handset has two ways of calling this action:

/gigaset?reqsrc=user&ln=foo   # Search for a contact with the name 'foo'
/gigaset?reqsrc=auto&hm=12345 # Search for a contact with caller ID 12345

Since both have different queries I decided to pattern match based on the reqsrc parameter. But both action have some shared logic as well, so I put that in a separate method with the same name but different arity to apply limit and offset and render the view/template. It works, but is this the right way of doing things?

  # User quering phonebook from Gigaset base
  def gigaset(conn, params = %{"reqsrc" => "user"}) do
    name = String.replace(params["ln"], "*", "%")
    query = from e in Entry, where: ilike(e.name, ^name)

    gigaset conn, query, params
  end

  # Caller ID from Gigaset base
  def gigaset(conn, params = %{"reqsrc" => "auto"}) do
    number = params["hm"]
    query = from e in Entry, where: e.work == ^number or e.home == ^number or e.mobile == ^number

    gigaset conn, query
  end

  # Finish query, render output
  def gigaset(conn, query, params) do
    limit = String.to_integer(params["count"])
    offset = String.to_integer(params["first"]) - 1

    query = from e in query, limit: ^limit, offset: ^offset
    entries = Repo.all(query)

    render conn, entries: entries
  end

I know the difference in the query is trivial, but imagine they would be vastly different. Does it make sense to do it like this?

2 Likes

Wait… after testing it again it seems to not work as expected :persevere:

Requesting /gigaset?reqsrc=auto yields a ‘no matching action clause to process request’ error on the definition of the first method. Is there a way to do this, at all?

I ended up rewriting into this long method (with some more changes):

  def gigaset(conn, params = %{ "reqsrc" => reqsrc, "count" => limit, "first" => offset }) do
    query = case reqsrc do
      # User quering phonebook from Gigaset base
      "user" ->
        name = String.replace(params["ln"], "*", "%")
        from e in Entry, where: ilike(e.name, ^name)

      # Caller ID from Gigaset base
      "auto" ->
        number = params["hm"]
        from e in Entry, where: e.work == ^number or e.home == ^number or e.mobile == ^number
    end

    # Total before pagination
    total = Repo.one(from e in query, select: count(e.id))

    # Paginated entries
    offset = String.to_integer(offset) - 1
    query = from e in query, limit: ^limit, offset: ^offset
    entries = Repo.all(query)

    render conn, entries: entries, total: total, offset: offset
  end

All feedback is welcome! I realise some of this belongs in the model probably.

1 Like

I don’t think there’s any problem in using pattern matching in controller methods. I’ve done this in one of my projects myself and it works great:

  def insert_attachment(conn, %{"ticket_message" => ticket_message,
                                "ticket_id"      => ticket_id}) do

    ticket = Repo.get(Ticket, ticket_id)
    upload = Map.get(ticket_message, "file")
    TicketAttachmentController.insert(conn, ticket, upload)
  end

  def insert_attachment(conn, %{"ticket_id"      => ticket_id}) do

    conn
    |> put_flash(:error, "No file was selected.")
    |> redirect(to: ticket_path(conn, :show, ticket_id))
  end
2 Likes

Your match does only “fire” when you have all the parameters "reqsrc", "count", and "first" available. In the requested “path” /gigaset?reqsrc=auto" there is only "reqsrc" available, so you need to add another clause after your current one, which needs to be like this:

def gigaset(conn, params = %{"reqsrc" => "auto"}), do: :stuff
2 Likes