Sorry for your job interview…

I have seen some part I would change, but given I don’t know the rule it’s kind of difficult to answer fully.

• Why start_server? I would use start_link
• Why use tuple when there is no argument? atom is enough I think
• Maintaining an id count is painful, if allowed, I would use UUID
• Why create your regex to parse host when there is URI module
• I would use cast for flush

I find also difficult to reason about the state you are passing.

As a simple example, I would store a simple tuple by given id, something like {host, url, count}

I don’t mean to have the perfect answer, but what about something like this?

  @impl GenServer
  def handle_call({:shorten, url}, _from, state) do
    host = URI.parse(url).host
    id = UUID.uuid4()
    new_state = Map.put(state, id, {host, url, 0})
    {:reply, id, new_state}
  end

  @impl GenServer
  def handle_call({:get, id}, _from, state) do
    case Map.get(state, id) do
      {host, url, count} ->
        {:reply, url, %{state | id => {host, url, count + 1}}}
      nil ->
        {:reply, nil, state}
    end
  end

  @impl GenServer
  def handle_call(:count, _from, state) do
    {:reply, Enum.count(state), state}
  end

  @impl GenServer
  def handle_call(:get_stats, _from, state) do
    reply = state
    |> Enum.group_by(fn {_k, {url, _, _}} -> url end)
    |> Enum.reduce(%{}, fn {key, list}, acc ->
      Map.put(acc, key, Enum.reduce(list, 0, fn {_k, {_, _, count}}, a -> a + count end))
    end)

    {:reply, reply, state}
  end

  @impl GenServer
  def handle_call(:get_click_stats, _from, state) do
    reply = state
    |> Enum.reduce(0, fn {_key, {_host, _url, count}}, acc -> acc + count end)
    {:reply, reply, state}
  end

  @impl GenServer
  def handle_call({:get_click_stats, id}, _from, state) do
    reply = state
    |> Enum.filter(fn {key, _value} -> key == id end)
    |> Enum.reduce(0, fn {_key, {_host, _url, count}}, acc -> acc + count end)
    {:reply, reply, state}
  end

  @impl GenServer
  def handle_cast(:flush, _state) do
    {:noreply, %{}}
  end

start_link is not only for historical reason, it means there is a link created between the parent and child processes, which is not the case with start.

And yes, our code makes about the same things

For the cast… it’s true I start with call everywhere, but when I really don’t expect any response, I switch for cast.