Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Switch function names #27

Merged
merged 1 commit into from
Apr 12, 2020
Merged

Conversation

milmazz
Copy link
Contributor

@milmazz milmazz commented Apr 12, 2020

The first time that I read this code:

  def validate(options, schema) do
    case validate_options_with_schema_and_path(schema, options_schema()) do

and this one:

  defp validate_options_with_schema(opts, fun, path) when is_function(fun) do
    validate_options_with_schema(opts, fun.(), path)

it confused me a bit, on the first validate_options_with_schema_and_path/2 I don't see the path as an argument, in the second block of code, I see that the path is needed but the function name is validate_options_with_schema. So, my best guess is that we should switch the function names and that's what I did here.

The first time that I read this code:

```elixir
  def validate(options, schema) do
    case validate_options_with_schema_and_path(schema, options_schema()) do
```

and this one:

```elixir
  defp validate_options_with_schema(opts, fun, path) when is_function(fun) do
    validate_options_with_schema(opts, fun.(), path)
```

it confuse me a bit, on the first `validate_options_with_schema_and_path/2` I
don't see the path as argument, in the second block of code, I see that the
path is needed but the function name is `validate_options_with_schema`. So, my
best guest is that we should switch the function names and that's what I did
here.
@whatyouhide whatyouhide merged commit c8452a5 into dashbitco:master Apr 12, 2020
@whatyouhide
Copy link
Collaborator

Good call, thanks @milmazz :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants