2
\$\begingroup\$
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 ?

\$\endgroup\$
3
\$\begingroup\$

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
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.