2016-05-21 16 views
1

Haben Sie Vorschläge, wie kann ich das sauberer machen?Aufräumen Elixir/Phoenix Karte Ersatzcode

Video Params ist von einer Formularvorlage, also eine Karte als% {"url" => "https://youtube.com/ ......", "title" => "Karneval in Rio de Janeiro", ...}

defp make_url_ready_for_embedding(video_params) do 
    cond do 
    String.contains? video_params["url"], "/watch?v=" -> 
     video_params |> Map.put("url", String.replace(video_params["url"], "/watch?v=", "/embed/")) 
    String.contains? video_params["url"], "https://vimeo.com" -> 
     video_params |> Map.put("url", Regex.replace(~r/([^1-9]+)/, video_params["url"], "https://player.vimeo.com/video/")) 
    true -> 
     video_params 
    end 
end 

Hier ist meine create Methode, wenn es von nutzen ist:

def create(conn, %{"video" => video_params}, user) do 
    changeset = 
     user 
     |> build_assoc(:videos) 
     |> Video.changeset(video_params |> make_url_ready_for_embedding) 

    case Repo.insert(changeset) do 
     {:ok, _video} -> 
     conn 
     |> put_flash(:info, "Video created successfully.") 
     |> redirect(to: video_path(conn, :index)) 
     {:error, changeset} -> 
     render(conn, "new.html", changeset: changeset) 
    end 
    end 

Antwort

2

der Sinn Logik machen, aber die Linien sind ziemlich lang.

ich wahrscheinlich für etwas gehen würde wie:

defp make_url_ready_for_embedding(%{"url" => url} = video_params) do 
    url = cond do 
    String.contains?(url, "/watch?v=") -> 
     String.replace(url, "/watch?v=", "/embed/") 
    String.contains?(url "https://vimeo.com") -> 
     Regex.replace(~r/([^1-9]+)/, url, "https://player.vimeo.com/video/") 
    true -> 
     url 
    end 
    %{video_params | "url" => url) 
end 

Dies ist besser, aber die Absicht ist noch nicht ganz klar auf einen Blick. Ich würde wahrscheinlich betrachten Funktionen mit diesem helfen:

defp make_url_ready_for_embedding(%{"url" => url} = video_params) do 
    type = cond do 
    youtube_video?(url) -> :youtube 
    vimeo_video?(url) -> :vimeo 
    true    -> :unknown 
    end 
    %{video_params | "url" => transform_url(url, type)} 
end 

defp youtube_video?(url) do 
    String.contains?(url, "/watch?v=") 
end 

defp vimeo_video?(url) do 
    String.contains?(url, "https://vimeo.com") 
end 

defp transform_url(url, :unknown) do: url 
defp transform_url(url, :youtube) do 
    String.replace(url, "/watch?v=", "/embed/") 
end 
defp transform_url(url, :vimeo) do 
    Regex.replace(~r/([^1-9]+)/, url, "https://player.vimeo.com/video/") 
end 

Dies hat den Vorteil, dass Sie Ihre transform_url Funktion testen können (es öffentlich und @doc false), um sicherzustellen, dass die URL für jeden Typ korrekt umgewandelt wird.

+0

Danke. Betrachte es jetzt .... – skovmand

+0

Ich denke, es ist schön. Ich habe die Funktionen in eine separate video_transformers.ex Datei verschoben und importiere sie jetzt – skovmand

+0

Es gibt einen Tippfehler in der defp 'youtube_video?/1' Funktion: ' String.contains? (Url "https://vimeo.com") 'sollte ein Komma haben, damit es:' String.contains? (url, "https://vimeo.com") ' –