# Refactor tips for simple function for more idiomatic Elixir

**URL:** <https://forum.elixirforum.com/t/refactor-tips-for-simple-function-for-more-idiomatic-elixir/12967>\
**Category:** Questions\
**Tags:** idiomatic-elixir\
**Created:** [March 8, 2018, 3:32pm UTC](https://forum.elixirforum.com/t/refactor-tips-for-simple-function-for-more-idiomatic-elixir/12967 "2018-03-08T15:32:11Z")\
**Posts on this page:** 10\
**Page:** 1

<div class="post-metadata">

**Author:** ![sergio](https://forum.elixirforum.com/user_avatar/forum.elixirforum.com/sergio/32/26966_2.png) [@sergio](https://forum.elixirforum.com/u/sergio)\
**Post date:** [March 8, 2018, 3:32pm UTC](https://forum.elixirforum.com/t/refactor-tips-for-simple-function-for-more-idiomatic-elixir/12967/1 "2018-03-08T15:32:11Z")

</div>

I’d love for Ecto.Repo.get or get\_by to return a `{:ok, foobar}` tuples. I could use it in `with` conditionals for cleaner more idiomatic Elixir.

Here’s my function:

```elixir
def can_list_bots?(team_id, user_id) do
  team = Repo.get(Team, team_id)
  user = Repo.get_by(User, id: user_id, team_id: team_id)

  if team != nil && user != nil do
    "admin" in user.roles
  else
    false
  end
end

```

Is there any way to make this code more idiomatic that I’m not seeing?

Thanks for the tips!

---

<div class="post-metadata">

**Author:** ![idi527](https://forum.elixirforum.com/letter_avatar_proxy/v4/letter/i/85e7bf/32.png) [@idi527](https://forum.elixirforum.com/u/idi527)\
**Post date:** [March 8, 2018, 3:48pm UTC](https://forum.elixirforum.com/t/refactor-tips-for-simple-function-for-more-idiomatic-elixir/12967/2 "2018-03-08T15:48:48Z")

</div>

Maybe

```elixir
def can_list_bots?(team_id, user_id) do
  team = Repo.get(Team, team_id)
  user = team && Repo.get_by(User, id: user_id, team_id: team_id)

  if user do
    "admin" in user.roles
  else
    false
  end
end

```

or if you want `with`

```elixir
def can_list_bots?(team_id, user_id) do
  with team when not is_nil(team) <- Repo.get(Team, team_id),
       user when not is_nil(user) <- Repo.get_by(User, id: user_id, team_id: team_id) do
    "admin" in user.roles
  else
    _ -> false
  end
end

```

or `with` with pattern matches

```elixir
def can_list_bots?(team_id, user_id) do
  with %Team{} <- Repo.get(Team, team_id),
       %User{roles: roles} <- Repo.get_by(User, id: user_id, team_id: team_id) do
    "admin" in roles
  else
    _ -> false
  end
end

```

You can also do it in a single sql query. Probably.

---

<div class="post-metadata">

**Author:** ![peerreynders](https://forum.elixirforum.com/user_avatar/forum.elixirforum.com/peerreynders/32/5826_2.png) [@peerreynders](https://forum.elixirforum.com/u/peerreynders)\
**Post date:** [March 8, 2018, 3:50pm UTC](https://forum.elixirforum.com/t/refactor-tips-for-simple-function-for-more-idiomatic-elixir/12967/3 "2018-03-08T15:50:58Z")

</div>

[with/1](https://hexdocs.pm/elixir/Kernel.SpecialForms.html#with/1) can use guards

```
team when team != nil <- Repo.get(Team, team_id)

```

---

<div class="post-metadata">

**Author:** ![kokolegorille](https://forum.elixirforum.com/user_avatar/forum.elixirforum.com/kokolegorille/32/4784_2.png) [@kokolegorille](https://forum.elixirforum.com/u/kokolegorille)\
**Post date:** [March 8, 2018, 3:56pm UTC](https://forum.elixirforum.com/t/refactor-tips-for-simple-function-for-more-idiomatic-elixir/12967/4 "2018-03-08T15:56:57Z")

</div>

Maybe I would start by removing if…

```elixir
def can_list_bots?(team_id, user_id) do
  team = Repo.get(Team, team_id)
  user = Repo.get_by(User, id: user_id, team_id: team_id)
  do_can_list_bots?(team, user)
end

defp do_can_list_bots?(nil, _), do: false
defp do_can_list_bots?(_, nil), do: false
defp do_can_list_bots?(_, user), do: "admin" in user.roles

```

But I don’t pretend to be a syntax expert, and the code is not tested.

I agree using _ **with** _ is the Elixir way.

---

<div class="post-metadata">

**Author:** ![peerreynders](https://forum.elixirforum.com/user_avatar/forum.elixirforum.com/peerreynders/32/5826_2.png) [@peerreynders](https://forum.elixirforum.com/u/peerreynders)\
**Post date:** [March 8, 2018, 3:58pm UTC](https://forum.elixirforum.com/t/refactor-tips-for-simple-function-for-more-idiomatic-elixir/12967/5 "2018-03-08T15:58:13Z")

</div>

> [@idi527](#):
>
> or `with` with pattern matches

This one gets my vote - especially on the `User` structure to snatch (destructure) the `roles` unless

> do it in a single sql query.

🙂

---

<div class="post-metadata">

**Author:** ![sergio](https://forum.elixirforum.com/user_avatar/forum.elixirforum.com/sergio/32/26966_2.png) [@sergio](https://forum.elixirforum.com/u/sergio)\
**Post date:** [March 8, 2018, 4:09pm UTC](https://forum.elixirforum.com/t/refactor-tips-for-simple-function-for-more-idiomatic-elixir/12967/6 "2018-03-08T16:09:44Z")

</div>

Thanks for the tips guys! I really like the with guard clause with `when not is_nil`.

---

<div class="post-metadata">

**Author:** ![axelson](https://forum.elixirforum.com/user_avatar/forum.elixirforum.com/axelson/32/26351_2.png) [@axelson](https://forum.elixirforum.com/u/axelson)\
**Post date:** [March 8, 2018, 6:25pm UTC](https://forum.elixirforum.com/t/refactor-tips-for-simple-function-for-more-idiomatic-elixir/12967/7 "2018-03-08T18:25:17Z")

</div>

Slightly off topic but do you really need to fetch the team at all? It seems like if you can assume that a team exists if the user has that team\_id then you don’t need to fetch the team at all.

---

<div class="post-metadata">

**Author:** ![aptinio](https://forum.elixirforum.com/user_avatar/forum.elixirforum.com/aptinio/32/456_2.png) [@aptinio](https://forum.elixirforum.com/u/aptinio)\
**Post date:** [March 8, 2018, 11:45pm UTC](https://forum.elixirforum.com/t/refactor-tips-for-simple-function-for-more-idiomatic-elixir/12967/8 "2018-03-08T23:45:03Z")

</div>

Haven’t tried it but maybe this single query could work:

```elixir
Repo.one(from u in User,
  where u.team_id == ^team_id,
  where u.id == ^user_id,
  where "admin" == fragment("ANY (?)", u.roles)
)

```

---

<div class="post-metadata">

**Author:** ![OvermindDL1](https://forum.elixirforum.com/user_avatar/forum.elixirforum.com/overminddl1/32/2677_2.png) [@OvermindDL1](https://forum.elixirforum.com/u/OvermindDL1)\
**Post date:** [March 8, 2018, 11:47pm UTC](https://forum.elixirforum.com/t/refactor-tips-for-simple-function-for-more-idiomatic-elixir/12967/9 "2018-03-08T23:47:09Z")

</div>

> [@aptinio](#):
>
> Haven’t tried it but maybe this single query could work:

+1

Yeah the more you can do in a single DB call, generally the better.

---

<div class="post-metadata">

**Author:** ![sorentwo](https://forum.elixirforum.com/user_avatar/forum.elixirforum.com/sorentwo/32/37360_2.png) [@sorentwo](https://forum.elixirforum.com/u/sorentwo)\
**Post date:** [March 9, 2018, 4:49am UTC](https://forum.elixirforum.com/t/refactor-tips-for-simple-function-for-more-idiomatic-elixir/12967/10 "2018-03-09T04:49:34Z")

</div>

Remember that your app’s Repo is still a module. Feel free to define a `fetch` function that converts `get` calls into success tuples.

In the primary app I work on there are `fetch*` functions that cover `get`, `one`, etc. All to make pattern matching in `with` chains clearer.