roger-melo-treinamentos / curso-de-js-roger-melo

Repositório de informações do CJRM
492 stars 172 forks source link

Aplicação: Weather App #2335

Closed Hamiltonspjunior closed 2 years ago

Hamiltonspjunior commented 2 years ago

As cores da sua versão da aplicação são diferentes das cores da aplicação mostrada na aula?

Sim

A aplicação contém funcionalidades que não foram mostradas nas aulas?

Não

A aplicação contém features da linguagem que não foram mostradas nas aulas?

Não

Link do repositório da aplicação

https://github.com/Hamiltonspjunior/weather-app

Maiores dificuldades durante a implementação

Menores dificuldades durante a implementação

Roger-Melo commented 2 years ago

Olá @Hamiltonspjunior.

Passando para dizer que visualizei a issue e se estiver tudo ok com a aplicação, em até 5 dias úteis a análise será postada aqui =)

Roger-Melo commented 2 years ago

Olá @Hamiltonspjunior!

Parabéns pelo esforço em construir a terceira aplicação do treinamento =)

Li as 5 dificuldades que vc listou, mas seu código mostra que você pegou muito bem os conceitos apresentados até aqui. Mesmo com as dificuldades, o código demonstra que vc conseguiu implementar muito bem a aplicação. Tanto é que a maioria dos ajustes que vou recomendar são opcionais =)

Vou deixar abaixo alguns pontos, segundo os critérios que usei para analisar.


Comentários

Não é uma regra, mas evite comentários.

Comentários podem mentir. Se o código for modificado e o comentário correspondente não for atualizado, o comentário perde o sentido. Isso sem mencionar o retrabalho de atualizar comentários.

Refatoração

Vc usou bem o ternário, pois não executou efeito colateral e aproveitou o valor que ele retorna.

Como não há interpolações, as strings do ternário não-necessariamente precisam ser template strings.

// antes
IsDayTime ? `./src/day.svg` : `./src/night.svg`
// depois
IsDayTime ? './src/day.svg' : './src/night.svg'

Isso é mais uma questão de code-styling do que performance ou algo do tipo, veja se faz sentido pra vc.


Opcional: vc pode deixar o código mais conciso e economizar algumas linhas se eliminar timeImageSrc e timeIconImg:

const getCityCardData = async inputValue => {
  const [{ Key, LocalizedName }] = await getCityData(inputValue)
  const [{ IsDayTime, Temperature, WeatherIcon, WeatherText }] = await getWeatherData(Key)

  return { LocalizedName, IsDayTime, Temperature, WeatherIcon, WeatherText }
}

const insertDataIntoCityCard = (cityCardData) => {
  const { LocalizedName, IsDayTime, Temperature, WeatherIcon, WeatherText } = cityCardData

  $timeImage.src = IsDayTime ? `./src/day.svg` : `./src/night.svg`
  $timeIcon.innerHTML = generateTimeIconImg(WeatherIcon, WeatherText)
  // ...
}

Outra opção que vc tem é eliminar insertDataIntoCityCard, mover código dela para dentro de handleSubmit e renomear handleSubmit para showCityWeather:

const showCityWeather = async event => {
  event.preventDefault()

  const inputValue = event.target.city.value
  const { LocalizedName, IsDayTime, Temperature, WeatherIcon, WeatherText } = await getCityCardData(inputValue)

  $timeImage.src = IsDayTime ? `./src/day.svg` : `./src/night.svg`
  $timeIcon.innerHTML = generateTimeIconImg(WeatherIcon, WeatherText)
  $cityName.innerHTML = LocalizedName
  $cityWeather.innerHTML = WeatherText
  $cityTemperature.innerHTML = Temperature.Metric.Value
  showCityCard()
  $cityForm.reset()
}

Assim vc concentra grande parte da manipulação do DOM em apenas uma função e deixa ela com um nome mais específico.

Nomes de variáveis e funções

Duas pequenas modificações que eu faria em relação à nomenclatura, são:


Nas aulas seguintes, você verá outras formas de refatorar o código dessa aplicação.

Mais uma vez, parabéns pelo esforço =)

As observações fizeram sentido?

Hamiltonspjunior commented 2 years ago

Oi, @Roger-Melo.

Fizeram sentido sim. Implementei as modificações que você sugeriu e vi que o código melhorou.

Gostei bastante de implementar essa aplicação por que deu pra perceber alguns conceitos que ainda não estavam muito bem entendidos mas que agora depois da prática estão.

Valeu pelo incentivo! Bora para a próxima etapa agora.

Roger-Melo commented 2 years ago

Excelente, Hamilton.

No que precisar, é só abrir uma nova issue. Conte comigo para chegar na fluência =)