Feedback Skilopay - Githubissues

Oi, Laila,

Primeiramente, parabéns pelo projeto, achei extremamente organizado, muito bem executado e completo. Estou impressionado! Pelo o que mostrou, não acho que vc esteja tão no início assim 😄

Mas, se eu fosse dar alguma dica (nada demais também), seria na função Cambio.converte_moeda, normalmente tento evitar IF dentro de IF. Eu particularmente acho um pouco mais difícil de entender e dar manutenção, mas isso é relativo, vai muito da experiência de quem está lendo seu código. Eu substituiria o IF pelo With ou colocaria aquela validação dentro uma função para dar mais semântica.

...
# usando with

  def converte_moeda(moeda_origem, moeda_destino, valor) do
    with true <- Map.has_key?(@conversoes, moeda_origem),
         true <- Map.has_key?(@conversoes[moeda_origem], moeda_destino) do
      {:ok, Decimal.new(Decimal.mult(valor, @conversoes[moeda_origem][moeda_destino]))}
    else
      _ -> {:error, :moeda_nao_suportada}
    end
  end

...

# ou dividindo em outra função

  def converte_moeda(moeda_origem, moeda_destino, valor) do
    if moedas_suportadas?(moeda_origem, moeda_destino) do
      {:ok, Decimal.new(Decimal.mult(valor, @conversoes[moeda_origem][moeda_destino]))}
    else
      {:error, :moeda_nao_suportada}
    end
  end

  defp moedas_suportadas?(moeda_origem, moeda_destino) do
    Map.has_key?(@conversoes, moeda_origem) &&
      Map.has_key?(@conversoes[moeda_origem], moeda_destino)
  end

É isso, você também sente que essa abordagem melhora o entendimento do código? Como você prefere?

No geral, achei muito bom o resultado, principalmente pq fez testes automatizados para o módulo Cli, essa não é uma preocupação tão comum. Parabéns!

Oi, Jonata!

De coração, obrigada pelo seu feedback.

Concordo com você que a abordagem do with e da divisão em outra função melhora o entendimento do código. Gostei muito das duas opções de código que vc escreveu. Só tenho uma ressalva, para quem é bem iniciante o with precisa de uma abstração maior, pelo menos é o que eu acho. Posso estar errada...

Quanto a questão da manutenção, eu concordo completamente contigo, IF dentro de IF tem uma manutenção mais penosa, evitarei usá-los :)

Jonata, não se engane, sou iniciante sim!!! Demorei 35 dias para fazer o teste que você viu. Esse foi meu primeiro e único teste até agora, antes disso eu só tinha feito exercícios e trabalhos para finalizar os cursos. Entretanto, eu me dedico demais em tudo que eu faço, gosto de estudar, sou super curiosa, sou persistente e acredito que eu consigo fazer qualquer coisa que eu me proponha a fazer, desculpa pela falta de modéstia rsrs.

Em relação a organização e fazer as coisas completas, sou assim em tudo que eu faço. Desde em arrumar minha cama até nos meus códigos.

Mais uma vez, muito obrigada! 🤗

On Fri, Oct 23, 2020 at 9:35 AM Jonata Weber notifications@github.com wrote:

Oi, Laila,

Primeiramente, parabéns pelo projeto, achei extremamente organizado, muito bem executado e completo. Estou impressionado! Pelo o que mostrou, não acho que vc esteja tão no início assim 😄

Mas, se eu fosse dar alguma dica (nada demais também), seria na função Cambio.converte_moeda, normalmente tento evitar IF dentro de IF. Eu particularmente acho um pouco mais difícil de entender e dar manutenção, mas isso é relativo, vai muito da experiência de quem está lendo seu código. Eu substituiria o IF pelo With ou colocaria aquela validação dentro uma função para dar mais semântica.

...

usando with

def converte_moeda(moeda_origem, moeda_destino, valor) do
with true <- Map.has_key?(@conversoes, moeda_origem),

     true <- Map.has_key?(@conversoes[moeda_origem], moeda_destino) do

  {:ok, Decimal.new(Decimal.mult(valor, @conversoes[moeda_origem][moeda_destino]))}

else

  _ -> {:error, :moeda_nao_suportada}

end
end

...

ou dividindo em outra função

def converte_moeda(moeda_origem, moeda_destino, valor) do
if moedas_suportadas?(moeda_origem, moeda_destino) do

  {:ok, Decimal.new(Decimal.mult(valor, @conversoes[moeda_origem][moeda_destino]))}

else

  {:error, :moeda_nao_suportada}

end
end

defp moedas_suportadas?(moeda_origem, moeda_destino) do
Map.has_key?(@conversoes, moeda_origem) &&

  Map.has_key?(@conversoes[moeda_origem], moeda_destino)
end

É isso, você também sente que essa abordagem melhora o entendimento do código? Como você prefere?

No geral, achei muito bom o resultado, principalmente pq fez testes automatizados para o módulo Cli, essa não é uma preocupação também comum assim. Parabéns!

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/LailaVasconcelos/desafio-stone/issues/1, or unsubscribe https://github.com/notifications/unsubscribe-auth/ALT25J7NCSAMK7FRTPRMSBDSMF2CDANCNFSM4S4QAXCA .

LailaVasconcelos / desafio-stone

Feedback Skilopay #1

usando with

ou dividindo em outra função