Reduce nested code in some functions #336
Conversation
| :error -> | ||
| Map.put(map, name, [{[version], parents}]) | ||
| end | ||
| end) | ||
| end | ||
|
|
||
| defp process_merge_package_version({:ok, list}, map, name, _, _, _), do: Map.put(map, name, list) | ||
| defp process_merge_package_version(:error, map, name, version, parents, list), |
There was a problem hiding this comment.
Function takes too many parameters (arity is 6, max is 5).
| :error -> | ||
| Map.put(map, name, [{[version], parents}]) | ||
| end | ||
| end) | ||
| end | ||
|
|
||
| defp process_merge_package_version({:ok, list}, map, name, _, _, _), do: Map.put(map, name, list) |
There was a problem hiding this comment.
Function takes too many parameters (arity is 6, max is 5).
| if Enum.sort(deps) != Enum.sort(Registry.deps(name, version)), | ||
| do: Mix.raise "Registry dependencies mismatch against lock (#{name} #{version})" | ||
| end | ||
| if deps, do: verify_dependencies(deps, name, version) |
There was a problem hiding this comment.
Function body is nested too deep (max depth is 2, was 3).
milmazz
force-pushed
the
reduce_function_body_nested_too_deep
branch
from
91ea205 to
80cc04e
Compare
|
|
||
| list | ||
| |> try_merge_package_version(version, parents) | ||
| |> process_merge_package_version(map, name, version, parents, list) |
There was a problem hiding this comment.
I think previously code was clearer, new function takes too many arguments.
| end) | ||
| end | ||
| end | ||
|
|
||
| defp show_status(nil, name, version), do: Hex.Shell.info IO.ANSI.format [:green, " #{name} #{version}"] |
There was a problem hiding this comment.
We usually prefix such functions with print_ instead of show_.
There was a problem hiding this comment.
Also there is too much things are happening inside this clause, I think we should use do ... end syntax.
| _ -> | ||
| :ok | ||
| end | ||
| end) | ||
| end | ||
|
|
||
| defp verify_registry(deps, name, version, checksum) do | ||
| registry_checksum = Registry.checksum(name, version) | ||
| if checksum && Base.decode16!(checksum, case: :lower) != registry_checksum, |
There was a problem hiding this comment.
do ... end syntax would be preferable here.
| {Atom.to_string(opts[:hex]), Atom.to_string(app), req, !!opts[:optional]} | ||
| end) | ||
|
|
||
| if Enum.sort(deps) != Enum.sort(Registry.deps(name, version)), |
There was a problem hiding this comment.
do ... end syntax would be preferable here.
| @@ -58,6 +53,9 @@ defmodule Hex.Crypto.PBES2_HMAC_SHA2 do | |||
| end | |||
| def decrypt(_, _, _, _), do: :error | |||
|
|
|||
| defp process_fetch_p2s({:ok, salt}, hash, passwd), do: {:ok, %{hash: hash, password: passwd}} | |||
There was a problem hiding this comment.
Maybe handle_p2s? process_fetch_p2s seems misleading.
There was a problem hiding this comment.
Thanks, I was looking for a better name in this function...
| content_encryptor_error -> | ||
| content_encryptor_error | ||
| end | ||
| protected |
There was a problem hiding this comment.
We can probably have fetch_content_encryptor function instead:
defp fetch_content_encryptor(key_manager, protected, opts) do
case ContentEncryptor.init(protected, opts) do
{:ok, content_encryptor} ->
{:ok, key_manager, content_encryptor}
error ->
error
end
There was a problem hiding this comment.
That also works, I'll change this section as you suggested.
milmazz
force-pushed
the
reduce_function_body_nested_too_deep
branch
from
80cc04e to
db9fbd3
Compare
|
Ebert has finished reviewing this Pull Request and has found:
You can see more details about this review at https://ebertapp.io/github/hexpm/hex/pulls/336. |
No description provided.