Phone validation ecto model

Question

defmodule Baby.Post do
  use Baby.Web, :model

  schema "posts" do
    field :cover, :string
    field :email, :string
    field :firstname, :string
    field :lastname, :string
    field :birthday_day, :integer
    field :birthday_month, :integer
    field :birthday_year, :integer
    field :description, :string
    field :phone, :string

    timestamps
  end

  @required_fields ~w(email firstname lastname birthday_day birthday_month birthday_year description phone)
  @optional_fields ~w()

  @doc """
  Creates a changeset based on the `model` and `params`.

  If no params are provided, an invalid changeset is returned
  with no validation performed.
  """
  def changeset(model, params \\ :empty) do
    model
    |> cast(params, @required_fields, @optional_fields)
    |> validate_length(:description, min: 280)
    |> validate_format(:email, ~r/@/)
    |> valid_phone(:phone)
  end


  @doc """
  Check if it's a valid french number
  """
  def valid_phone(changeset, field) do
    phone = get_field(changeset, field)

    unless phone === nil do

      if String.length(phone) !== 10  do
        add_error(changeset, field, "n'est pas valide")
      else
        changeset
      end

    else 
      changeset
    end

  end


end

I'm trying to add a custom validation for the phone field. The code is working, but it's really crappy. I'm quite new with elixir and I'm a bit confused about the fact the return is the last expression evaluated. Can you help me to rewrite that valid_phone method ?

Answer 1

When you end up with a pyramid of if-else clauses like this, it's sometimes a good idea to use cond instead. cond lets you specify a bunch of clauses, and picks the first one that matches.

For example:

def valid_phone(changeset, field) do
  phone = get_field(changeset, field)
  cond do
    phone == nil -> 
      changeset
    String.length(phone) != 10 -> 
      add_error(changeset, field, "n'est pas valide")
    true ->
      changeset
  end
end

Of course, that can be simplified further into

def valid_phone(changeset, field) do
  phone = get_field(changeset, field)
  cond do
    phone != nil and String.length(phone) != 10 -> 
      add_error(changeset, field, "n'est pas valide")
    true ->
      changeset
  end
end

Which can be simplified further, because any cond with one clauses & a true is really just a fancy if-else expression:

def valid_phone(changeset, field) do
  phone = get_field(changeset, field)
  if phone != nil && String.length(phone) != 10 do
    add_error(changeset, field, "n'est pas valide")
  else
    changeset
  end
end