fernandomk6 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?

Sim

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

Não

Link do repositório da aplicação (ou pasta pública no Google Drive)

https://github.com/fernandomk6/weather-application

Maiores dificuldades durante a implementação

Validar o retorno de uma função async

Visto que geralmente usamos async para fazer requisições e para tratar possíveis erros, usamos o bloco try catch. A aplicação espera que a função async retorne um response.json, mas caso ocorra um erro, a async execute o bloco catch, a função retornará uma promise pending com undefined encapsulado.

A maior dificuldade foi validar esse possível retorno undefined de uma função async quando o bloco catch for executado.

Também tive um pouco de dificuldade ao isolar responsabilidades de funções sobrecarregadas e em como nomear essas funções. Fiquei um pouco na dúvida se escolhi bons nomes para as const.

Menores dificuldades durante a implementação

Fazer funcionar,

Não tive dificuldade em fazer funcionar. As dificuldades foram em refatorar e tratar erros.

MivlaM commented 2 years ago

Olá @fernandomk6.

Eu abri a aplicação e identifiquei que o segundo critério para que a análise seja feita está em falta.

O critério é o seguinte:

Critério 2: A aplicação deve conter nenhuma funcionalidade à mais do que as que foram mostradas nas aulas anteriores.

Os critérios e regras para que a análise seja feita foram enviados no email "[Alunos CJRM] Análises de Aplicações".

Novas funcionalidades não devem ser adicionadas agora, o mais importante no momento que estamos analisando uma aplicação é ver se o aluno assimilou e conseguiu compreender bem o que foi falado até aqui. Foi pedido no enunciado do exercício e também descrito no email "[Alunos CJRM] Análise de aplicações" que a aplicação não tenha funcionalidades que não foram mostradas nas aulas.

Eu recomendo que você salve esse email em algum lugar e marque ele com estrela (no gmail). Por que ele é importante e você terá que voltar nele quando enviar as aplicações.

Considere manter em seu código apenas os trechos relativos às funcionalidades mostradas nas aulas da aplicação, solicito que por favor, remova as funcionalidades adicionais para que possamos prosseguir com o processo de análise de aplicação.

Avise por aqui quando fizer a mudança =)

Fico no aguardo.

fernandomk6 commented 2 years ago

As únicas funcionalidade a mais foi um trecho de css, onde foi colocado uma imagem que se repete. A nível js só foi usado o que tem nas aulas.

MivlaM commented 2 years ago

Olá @fernandomk6.

Desculpas por não ter sido específico na mensagem anterior, mas eu estava me referindo a funcionalidade adicional da mensagem de carregamento inserida na aplicação ou seja, a função showLoadingMessage. Exibir uma mensagem de carregamento após realizar uma ação é uma funcionalidade adicional para essa aplicação, você tem mérito por ter desenvolvido uma nova funcionalidade e você ainda vai poder aplicá-la futuramente, mas, neste momento, para a análise, solicito-lhe que ela seja removida para analisarmos o foco principal do conteúdo ensinado na etapa =)

fernandomk6 commented 2 years ago

Essa funcionalidade apenas usa o que já foi mostrado nas aulas, inserir um elemento no DOM e inserir um texto nesse elemento.

Se o esperado é que seja feito uma réplica da aplicação desenvolvida em aula não faz o menor sentido uma análise.

O interessante das aplicações no meu ponto de vista é tentar desenvolver algo com as próprias mãos e se poder adicionar algo (que já foi falado nas aulas) e fizer sentido, eu vejo isso como um beneficio. Não faz nenhum sentido "copiar" o código da aula.

Mas tudo bem entendo o processo, obrigado.

Roger-Melo commented 2 years ago

Olá @fernandomk6!

Se o esperado é que seja feito uma réplica da aplicação desenvolvida em aula não faz o menor sentido uma análise.

Após analisar centenas de aplicações, eu posso dizer que faz sentido.

Por que é o seguinte...

O que estamos analisando é se a pessoa consegue construir a aplicação sozinha, como foi recomendado no email [Alunos CJRM] Bônus: Análise de aplicações.

Saca só essa parte do email:

"Se você viu as aulas de como construir a aplicação de Quiz, por exemplo, você tem 7 dias para implementar essa aplicação *sozinho(a).

*O que eu quero dizer com 'sozinho' é:

Sem assistir as aulas da aplicação novamente, implemente-a.
Se travar em um ponto, assista a aula, apague todo o código e implemente-o novamente, sozinho. Se não conseguir implementar sem assistir a aula, veja-a novamente. Após sanar a dúvida, apague o código e implemente-o, sozinho.
Execute o item acima quantas vezes forem necessárias para que seu cérebro entenda o padrão de implementação da aplicação. Isso te deixará mais "afiado" para os próximos desafios do CJRM.

E claro, se mesmo executando os itens acima as dúvidas permanecerem, não hesite em abrir uma issue no repositório do CJRM."

O importante pra gente é analisar o essencial que foi mostrado para a aplicação. Por que se a pessoa consegue construir a aplicação sozinha, fica meio que implícito que provavelmente, ela consegue fazer adições de funcionalidades (afinal, ela já fez o mais difícil).

Você não faz ideia do quanto as pessoas improvisam antes de aprender fazer o fundamental e acabam fazendo nem o improviso nem o fundamental bem feito (não estou dizendo que é esse o seu caso).

Mas isso acontece por que as pessoas não tropeçam em montanhas. Elas tropeçam em pequenas pedras.

Por isso, em uma análise, a gente se importa mais em checar se a pessoa realmente aprendeu os fundamentos do que analisar improvisos =)

Se a gente percebe que a pessoa realmente sacou os fundamentos, nós somos os primeiros a incentivar melhorias. E pelas suas respostas às análises passadas das suas aplicações, eu sinto que você percebeu isso.

Dito isso, como a feature que vc adicionou é bem básica, a aplicação será analisada. Nos próximos envios, apenas tome essa precaução que mencionei acima =)

fernandomk6 commented 2 years ago

Entendi, nas próximos farei exatamente como estão nas aulas. Eu interpretei a mensagem de forma diferente.

A aplicação deve conter nenhuma funcionalidade à mais do que as que foram mostradas nas aulas anteriores.

Interpretei como: "Não usar nenhum funcionalidade que não foi mostrada até agora no curso". Mas pelo que você disse é: "Não usar nenhum funcionalidade que não foi usada na aplicação modelo".

Entendi a mensagem as próximas seguirão esse critério.

MivlaM commented 2 years ago

Olá @fernandomk6.

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

A maior dificuldade foi validar esse possível retorno undefined de uma função async quando o bloco catch for executado.

Vamos falar sobre isso no decorrer da análise.

Também tive um pouco de dificuldade ao isolar responsabilidades de funções sobrecarregadas e em como nomear essas funções. Fiquei um pouco na dúvida se escolhi bons nomes para as const.

Também será abordado na análise =)

Vou deixar abaixo algumas observações sobre sua aplicação.

Nomenclatura

Se fizer sentido para você, a palavra "ChangeLocation" que você usou na const formChangeLocation pode ser removida e o nome da const ser encurtada em apenas form, visto que há apenas um único form na aplicação. O form é um elemento que dificilmente vai mudar de elemento na aplicação. Assim já é o suficiente para quem está lendo entender que a const form está armazenando a referência do form.

// antes
const formChangeLocation = document.querySelector('[data-js="change-location"]')

// depois
const form = document.querySelector('[data-js="change-location"]')

Para a const h5CityLocation, se futuramente o elemento mudar para um h3, por exemplo, o nome da const vai perder o sentido. O mesmo vale para as consts spanTemperature, divWeatherText, divTimeIcon, divWeatherCard, spanLoading, elas podem ser renomeadas de forma que elementos propensos a serem mudados na marcação html não apareçam na nomenclatura. Veja as sugestões abaixo:

// antes
const h5CityLocation = document.querySelector('[data-js="city-location"]')
const spanTemperature = document.querySelector('[data-js="temperature"]')
const divWeatherText = document.querySelector('[data-js="weather-text"]')
const divTimeIcon = document.querySelector('[data-js="time-icon"]')
const divWeatherCard = document.querySelector('[data-js="weather-card"]')
const spanLoading = document.querySelector('[data-js="loading"]')

// depois
const headingLocation = document.querySelector('[data-js="city-location"]')
const temperatureValue = document.querySelector('[data-js="temperature"]')
const weather = document.querySelector('[data-js="weather-text"]')
const timeIcon = document.querySelector('[data-js="time-icon"]')
const weatherCard = document.querySelector('[data-js="weather-card"]')
const loadingMessage = document.querySelector('[data-js="loading"]')

Callback de envio do form

A ideia aqui é a seguinte: a ação mais significativa que acontece na aplicação ao enviar o form pode ser ficar mais descritiva e ainda facilitaria a legibilidade se o callback do envio do form for nomeado por algo como showCityWeather, por exemplo.

// antes
form.addEventListener('submit', async (event) => {
  event.preventDefault()
  // ...
})

// depois
const showCityWeather = async (event) => {
  event.preventDefault()
  // ...
}

form.addEventListener('submit', showCityWeather)

Agora, vale a pena se atentar a nomenclatura de weatherCardInfo e da função getWeatherCardInfo. Para facilitar a leitura do código e deixá-lo menos extenso, penso que pode ser interessante renomeá-los para weather e getWeatherInfo, respectivamente.

// antes
const getWeatherCardInfo = async (cityName) => {
  // ...
}

const showCityWeather = async (event) => {
  // ...
  const weatherCardInfo = getWeatherCardInfo(cityName)
  // ...
}

// depois
const getWeatherInfo = async (cityName) => {
  // ...
}

const showCityWeather = async (event) => {
  // ...
  const weather = getWeatherInfo(cityName)
  // ...
}

Refatoração de funções

Na função showLoadingMessage, podemos fazer algumas modificações. Primeiramente, ela está sendo declarada depois de já ter sido invocada em outras funções, para facilitar o fluxo da leitura, uma sugestão seria declará-la antes de ela ser usada.

// antes
const getWeatherInfo = async (cityName) => {
  showLoadingMessage('Buscando clima na cidade')

  // ...
}

const showLoadingMessage = (message) => {
  // ... 
}

// depois
const showLoadingMessage = (message) => {
  // ... 
}

const getWeatherInfo = async (cityName) => {
  showLoadingMessage('Buscando clima na cidade')

  // ...
}

Também é possível, se você achar interessante, encurtar uma linha de código e resumir essa função em uma única linha. Para isso, podemos omitir os blocos da arrow function e mantê-la em uma linha. Ficaria assim:

// antes
const showLoadingMessage = (message) => {
  loadingMessage.textContent = message || '' 
}

// depois
const showLoadingMessage = (message) => loadingMessage.textContent = message || ''

Se você quiser deixar a função um pouco mais simples, você pode remover esse curto circuito e na invocação dessa função que recebe false, você pode fazer com que ela receba uma string vazia. Dessa forma, a função não precisa mais de um curto circuito para receber um boolean como argumento, recebendo apenas string, ela precisaria apenas lidar com esse tipo de valor e teoricamente, lidar com um tipo de valor seria mais fácil que lidar com dois.

// antes
const showLoadingMessage = (message) => loadingMessage.textContent = message || '' 

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

  showLoadingMessage(false)

  // ...
}

// depois
const showLoadingMessage = (message) => loadingMessage.textContent = message

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

  showLoadingMessage('')

  // ...
}

Agora, uma forma de simplificar a obtenção dos dados do clima é invocar apenas uma função no app.js que possa retornar esses dados. Mas para isso acontecer, primeiro será necessário uma refatoração nas funções do weather.js. O que quero mostrar aqui, é que não é necessário ter duas funções getCityData e getCityWeatherData quando as únicas informações que mudam de uma função para outra são a URL e a mensagem do erro de response.ok, e essas informações podem ser recebidas como parâmetro. A ideia aqui é reduzir a quantidade de linhas e deixar o código mais prático, e visto que sempre haverão dois requests para pegar as informações do clima, pode haver apenas uma única função para fazer os requests. Não sendo necessário repetir o try/catch, if, response.json(), etc. Já que a função vai receber dois parâmetros, para facilitar na legibilidade, vamos fazer os parâmetros ficarem descritivos o suficiente por meio de um objeto como argumento dessa função, e as propriedades do objeto vão descrever o que esses valores representam, ficaria assim:

// antes
const getCityData = async (cityName) => {
  try {
    const response = await fetch(`${baseURL}/locations/v1/cities/search?apikey=${APIKey}&q=${cityName}&language=${language}`)

    if (!response.ok) {
      throw new Error('Não foi possível obter os dados da cidade')
    }

    return response.json()

  } catch (error) {
    console.log(error.message)
  }
}

const getCityWeatherData = async (locationKey) => {
  try {
    const response = await fetch(`${baseURL}/currentconditions/v1/${locationKey}?apikey=${APIKey}&language=${language}`)

    if (!response.ok) {
      throw new Error('Não foi possível obter os dados do clima')
    }

    return response.json()
  } catch (error) {
    console.log(error.message)
  }
}

// depois
const fetchData = async ({ url, errorMessage }) => {
  try {
    const response = await fetch(url)

    if (!response.ok) {
      throw new Error(errorMessage)
    }

    return response.json()
  } catch (error) {
    console.log(error.message)
  }
}

const getCityData = cityName => fetchData({ 
  url: `${baseURL}/locations/v1/cities/search?apikey=${APIKey}&q=${cityName}&language=${language}`, 
  errorMessage: 'Não foi possível obter os dados da cidade' })

const getCityWeatherData = locationKey => fetchData({ 
  url: `${baseURL}/currentconditions/v1/${locationKey}?apikey=${APIKey}&language=${language}`, 
  errorMessage: 'Não foi possível obter os dados do clima' })

Após declarar essa função fetchData, o weather.js pode ter uma função "x" (pode ser qualquer nome por enquanto), que recebe o nome da cidade como argumento e retorna o objeto com as propriedades necessárias. Esse objeto que vai ser retornado, pode ser o que está sendo retornado dentro do try da função getWeatherInfo no app.js. Seria algo próximo disso:

const x = async (cityName) => {
  try {
    const [{ Key, LocalizedName, AdministrativeArea, Country }] = await getCityData(cityName)
    const [{ IsDayTime, Temperature, WeatherIcon, WeatherText }] = await getCityWeatherData(Key)

    return {
      city: LocalizedName,
      temperature: Temperature.Metric.Value,
      weatherText: WeatherText,
      isDayTime: IsDayTime,
      weatherIcon: WeatherIcon,
      country: Country.LocalizedName, 
      state: AdministrativeArea.ID
    }
  } catch (error) {
    console.log(error.message)
    showLoadingMessage('Cidade não encontrada')
  }
}

Feito isso, no objeto, há uma propriedade nomeada como "cityName". Então, com o intuito de não causar confusão com o nome do parâmetro da função x (que também é cityName), vamos trocar o nome da propriedade para city, lembrando de fazer o mesmo em cityLocation e weather. Agora, para deixarmos o código mais claro e facilitar na legibilidade, ao invés de "x", essa função pode ser nomeada como getWeatherInfo. Ficaria assim:

const getWeatherInfo = async cityName => {
  try {
    const [{ Key, LocalizedName, AdministrativeArea, Country }] = await getCityData(cityName)
    const [{ IsDayTime, Temperature, WeatherIcon, WeatherText }] = await getCityWeatherData(Key)

    return {
      city: LocalizedName,
      temperature: Temperature.Metric.Value,
      weatherText: WeatherText,
      isDayTime: IsDayTime,
      weatherIcon: WeatherIcon,
      country: Country.LocalizedName, 
      state: AdministrativeArea.ID
    }
  } catch (error) {
    console.log(error.message)
    showLoadingMessage('Cidade não encontrada')
  }
}

Tudo certo até aqui? Se sim, vamos prosseguir, atente-se ao fato de que há uma getWeatherInfo no app.js, então por enquanto comente ela, pois vamos precisar ver em breve a questão das invocações de showLoadingMessage. Após comentar a antiga getWeatherInfo do app.js, getWeatherInfo já estará invocada dentro de showCityWeather, mas vamos colocar o await antes da invocação porque se trata de uma função assíncrona agora, junto disso, vamos mover o código que manipula o DOM armazenado dentro de buildWeatherCard para showCityWeather. Feito isso, essa função buildWeatherCard não precisará mais existir. Ficaria assim:

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

  const cityName = event.target.city.value
  const weather = await getWeatherInfo(cityName)

  const { 
    city,
    temperature,
    weatherText,
    isDayTime,
    weatherIcon,
    state,
    country
  } = weather

  const cityLocation = `${cityName}, ${state} - ${country}`
  headingLocation.textContent = cityLocation
  temperatureValue.textContent = temperature
  weather.textContent = weatherText

  setImgTime(isDayTime)
  setTimeIcon(weatherIcon)

  // ...
}

Um detalhe adicional, é que a showLoadingMessage da primeira linha da antiga getWeatherInfo pode ser movida para dentro da função showCityWeather, acredito que faça sentido exibir essa mensagem assim que o form foi enviado.

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

  const cityName = event.target.city.value
  const weather = await getWeatherInfo(cityName)

  // ...
}

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

  showLoadingMessage('Buscando clima na cidade')

  const cityName = event.target.city.value
  const weather = await getWeatherInfo(cityName)

  // ...
}

Uma outra sugestão de ajuste, se fizer sentido para você, é remanejar o código de cityLocation para onde ele é usado e a const cityLocation pode ser eliminada. Acredito que assim, você economizaria uma linha e não vejo necessidade de armazenar esse trecho numa const que só vai ser usada uma vez logo abaixo de sua declaração. A função setTimeIcon também pode ser eliminada e seu código colocado na showScore, o código de setTimeIcon não é uma expressão ilegível à ponto de precisar encapsular o código em uma função para dar mas legibilidade.

// antes
const showCityWeather = async (event) => {
  // ...

  const cityLocation = `${cityName}, ${state} - ${country}`
  headingLocation.textContent = cityLocation
  temperatureValue.textContent = temperature
  weather.textContent = weatherText

  setImgTime(isDayTime)
  setTimeIcon(weatherIcon)

  // ...
}

// depois
const showCityWeather = async (event) => {
  // ...

  headingLocation.textContent = `${city}, ${state} - ${country}`
  temperatureValue.textContent = temperature
  weather.textContent = weatherText

  setImgTime(isDayTime)

  const imgTimeIconHTML = `<img src="./src/icons/${weatherIcon}.svg"/>`
  timeIcon.innerHTML = imgTimeIconHTML

  // ...
}

Algo a adicionar é que, ainda dentro da função showCityWeather, a função showWeatherCard também não precisa existir e pode ter seu código remanejado para o final da função, um detalhe importante aqui, é que o método remove() pode ser invocado sem verificação, porque se a classe não existe no elemento, nada acontece, nenhum erro é lançado. Ficaria assim:

// antes
const showWeatherCard = () => {
  const isVisible = !weatherCard.classList.contains('d-none')
  !isVisible && weatherCard.classList.remove('d-none')
}

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

  showWeatherCard()

  event.target.reset()
}

// depois
const showCityWeather = async (event) => {
  // ...

  weatherCard.classList.remove('d-none')

  event.target.reset()
}

Sobre showLoadingMessage('Cidade não encontrada'), você poderia por exemplo, dentro da função getWeatherInfo, fazer uma validação do array que a invocação de fetchData retorna. Se o array está vazio, isso significa que não há no array objeto que represente alguma cidade (se você quiser, pode testar o retorno da API nesse form: https://developer.accuweather.com/accuweather-locations-api/apis/get/locations/v1/cities/search). Nesse caso, a ideia aqui é usarmos um return dentro do if para que a função pare de ser executada. Ficaria assim:

// antes
const getWeatherInfo = async cityName => {
  try {

    const [{ Key, LocalizedName, AdministrativeArea, Country }] = await getCityData(cityName)
    // ...

  } catch (error) {
    console.log(error.message)
    showLoadingMessage('Cidade não encontrada')
  }
}

// depois
const getWeatherInfo = async cityName => {
  try {

    const cityData = await getCityData(cityName)

    if (!cityData.length){
      return
    }

    const [{ Key, LocalizedName, AdministrativeArea, Country }] = cityData
    // ...

  } catch (error) {
    console.log(error.message)
    showLoadingMessage('Cidade não encontrada')
  }
}

Porém, só isso não é suficiente, porque dentro de showCityWeather, existe a invocação de getWeatherInfo. E se o return dentro do if foi executado, essa invocação vai retornar undefined. Por isso, antes de fazer destructuring no retorno da invocação de getWeatherInfo, também vai ser preciso validar. Se o retorno for undefined, a ideia aqui é exibir a mensagem de cidade não encontrada e interromper a execução de showCityWeather.

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

  showLoadingMessage('Buscando clima na cidade')

  const cityName = event.target.city.value
  const weather = await getWeatherInfo(cityName)

  const { 
    city,
    temperature,
    weatherText,
    isDayTime,
    weatherIcon,
    state,
    country
  } = weather

  // ...
}

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

  showLoadingMessage('Buscando clima na cidade')

  const cityName = event.target.city.value
  const weather = await getWeatherInfo(cityName)

  if (!weather){
    showLoadingMessage('Cidade não encontrada')
    return
  }

  const { city, temperature, weatherText, isDayTime, weatherIcon, state, country } = weather

  //...
}

Se showLoadingMessage exibe uma mensagem como "Cidade não encontrada", pode não fazer muito sentido chamá-la de showLoadingMessage. A sugestão aqui, se você achar interessante, é renomeá-la para algo como showMessage, o que pode fazer mais sentido em relação a mensagem que ela passa.

// antes
const showLoadingMessage = (message) => loadingMessage.textContent = message

showLoadingMessage('Cidade não encontrada')

// depois
const showMessage = (message) => loadingMessage.textContent = message

showMessage('Cidade não encontrada')

Por fim, conseguimos reduzir mais de 30 linhas de código por meio dessas refatorações. 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?

fernandomk6 commented 2 years ago

Fizeram sim, muito obrigado.

MivlaM commented 2 years ago

Show! Vou fechar a issue mas no que precisar, é só abrir uma nova =)

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

Aplicação: weather application #5444

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

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

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

Link do repositório da aplicação (ou pasta pública no Google Drive)

Maiores dificuldades durante a implementação

Validar o retorno de uma função async

Menores dificuldades durante a implementação

Critério 2: A aplicação deve conter nenhuma funcionalidade à mais do que as que foram mostradas nas aulas anteriores.

Se o esperado é que seja feito uma réplica da aplicação desenvolvida em aula não faz o menor sentido uma análise.

A maior dificuldade foi validar esse possível retorno undefined de uma função async quando o bloco catch for executado.

Também tive um pouco de dificuldade ao isolar responsabilidades de funções sobrecarregadas e em como nomear essas funções. Fiquei um pouco na dúvida se escolhi bons nomes para as const.

Nomenclatura

Callback de envio do form

Refatoração de funções