Aplicação: Todo List - Githubissues

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?

Sim

Link do repositório da aplicação

https://github.com/danielscastro/cjrm-todo-list

Maiores dificuldades durante a implementação

Tive grande dificuldade, pesquisei, procurei solução para refatorar a deleção dos itens usando datasets, mas não consegui. No entanto, durante o processo, encontrei uma forma alternativa que refatora o código e resolve o problema de uma possível mudança no HTML, utilizando o método closest. Durante minhas pesquisas também encontrei uma implementação sugerida por você no filtro dos todos, que retorna um array de objetos, com o todo e um boolean se ele deve ser exibido ou não. "Pesquei" uma pouco essa resposta, mas fiz algumas alterações na minha implementação.

Percebo que, por conta da falta de familiaridade e treino, eu tenho certa resistência ao ato de refatorar, bem como acho difícil algumas vezes. Estou me esforçando para mexer e alterar, às vezes de forma desnecessária, só pra treinar esse aspecto.

Menores dificuldades durante a implementação

Seletores, evento target, manipulações básicas da DOM e a maioria dos assuntos abordados já estão bem consolidados na minha mente.

Olá @danielscastro.

Passando para dizer que visualizei a issue e em até 5 dias úteis a análise será postada aqui =)

Olá @danielscastro.

Parabéns pelo esforço em construir a segunda aplicação do treinamento!

Legal que vc pesquisou outras issues de aplicações e o método closest. Os que fazem isso são minoria. Fiquei muito feliz com sua atitude =)

Tive grande dificuldade, pesquisei, procurei solução para refatorar a deleção dos itens usando datasets, mas não consegui. No entanto, durante o processo, encontrei uma forma alternativa que refatora o código e resolve o problema de uma possível mudança no HTML, utilizando o método closest. Durante minhas pesquisas também encontrei uma implementação sugerida por você no filtro dos todos, que retorna um array de objetos, com o todo e um boolean se ele deve ser exibido ou não. "Pesquei" uma pouco essa resposta, mas fiz algumas alterações na minha implementação.

Vou falar sobre isso ali em baixo.

Percebo que, por conta da falta de familiaridade e treino, eu tenho certa resistência ao ato de refatorar, bem como acho difícil algumas vezes. Estou me esforçando para mexer e alterar, às vezes de forma desnecessária, só pra treinar esse aspecto.

Sim, sempre experimente com o código.

E ao mesmo tempo, tenha consciência que refatoração é uma habilidade que vai sendo elevada conforme vc pratica. E vc ainda praticará bastante durante o treinamento e será exposto há vários casos de uso.

Mas o mais importante num primeiro momento vc já fez, que é fazer funcionar. Refatoração vem depois =)

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

O parâmetro `e`

Em addNewTodo, um parâmetro e foi declarado mas a expressão event.target.add.value.trim() não está usando o parâmetro.

Isso significa que a expressão está usando o event global.

Mesmo que tenha funcionado, o ideal é que o e seja usado na expressão. Isso por que o event global foi descontinuado. O VSCode geralmente insere um traço no meio de propriedades e métodos que foram descontinuados.

Quando você insere o event como parâmetro de um callback de listener de evento e usa ele, ao invés de usar o event global, você está usando o Event interface =)

// antes
const inputValue = event.target.add.value.trim()

// depois
const inputValue = e.target.add.value.trim()

`searchTodos`

Se vc quiser, o código da searchTodos pode ficar mais conciso.

Basta encadear a invocação do map diretamente em Array.from():

// antes
const searchTodos = e => {
  const inputValue = e.target.value.trim().toLowerCase()
  const todos = Array.from(todosContainer.children)

  const todoWithHiddenValue = todos
    .map(todo => ({
      todo,
      hidden: !todo.textContent.toLowerCase().includes(inputValue)
    }))

  manageSearchedTodos(todoWithHiddenValue)
}

// depois
const searchTodos = e => {
  const inputValue = e.target.value.trim().toLowerCase()
  const todos = Array
    .from(todosContainer.children)
    .map(todo => ({
      todo,
      hidden: !todo.textContent.toLowerCase().includes(inputValue)
    }))

  manageSearchedTodos(todos)
}

Possível refactoring para melhorar a legibilidade

Um princípio que gosto de seguir ao nomear é:

Funções são verbos que descrevem a ação que a função executa.
- Exemplos: getName, setName, incrementCounter;
Variáveis/consts/propriedades são substantivos que descrevem o valor armazenado.
- Exemplos: user, score, color.
- Quando o valor armazenado é um boolean, podem conter is, has ou should.
- Exemplos: isOdd, hasSevenItems, shouldUpdate.

Considerando isso...

Se a propriedade hidden for renomeada para shouldBeVisible, fica mais claro que ela armazena um boolean.

// antes
{
  todo,
  hidden: !todo.textContent.toLowerCase().includes(inputValue)
}

// depois
{
  todo,
  shouldBeVisible: todo.textContent.toLowerCase().includes(inputValue)
}

Um outro detalhe nessa modificação é que é mais fácil ler e interpretar uma afirmação do que uma negação.

Não pense em um elefante rosa.

Vc pensou no elefante rosa, certo?

Isso aconteceu pq nosso cérebro lida melhor com afirmações. Sempre que possível, use afirmações em suas expressões ou condições.

Obviamente, após fazer isso, vc precisará modificar forEach e ternário que insere ou remove as classes CSS:

// antes
const manageSearchedTodos = todos => {
  todos.forEach(({ todo, hidden }) => {
      todo.classList.add(hidden ? 'hidden' : 'd-flex')
      todo.classList.remove(hidden ? 'd-flex' : 'hidden')
    })
}

// depois
const manageSearchedTodos = todos => {
  todos.forEach(({ todo, shouldBeVisible }) => {
    todo.classList.add(shouldBeVisible ? 'd-flex' : 'hidden')
    todo.classList.remove(shouldBeVisible ? 'hidden' : 'd-flex')
  })
}

Omissão de blocos em `if`s

Cuidado ao omitir blocos em ifs, como vc fez aqui:

const deleteTodo = e => {
  const clickedElement = e.target

  if (Array.from(clickedElement.classList).includes('delete'))
    clickedElement.closest('li').remove()
}

Esse bug crítico no SSL da Apple aconteceu há alguns anos e colocou a segurança de dados em xeque por causa desse tipo de decisão.

Legibilidade em condicionais

Evite usar uma expressão extensa como condição.

O que vc acha mais legível?

// antes
if (Array.from(clickedElement.classList).includes('delete'))

// depois
if (clickedElementWasTrash)

`closest`

Ao fazer manipulação de DOM como no código abaixo, o ideal é que ao invés de buscar pela tag do elemento, vc busque por um identificador que ele tem.

const deleteTodo = e => {
  // ...
  if (clickedElementWasTrash) {
    clickedElement.closest('li').remove()
  }
}

Isso pq se, por acaso, a tag do elemento for modificada (li para div ou algo do tipo), o código acima quebra.

Esse é um dos motivos pelos quais nas próximas aulas, vc conhecerá o atributo data-* e a propriedade dataset.

Como o closest recebe um seletor CSS, vc pode testar usar ele ao invés do dataset. Mas o fundamento aqui é evitar buscar pela tag.

Em aplicações onde há mudanças mais intensas de DOM, buscar pela tag pode quebrar seu código facilmente.

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?

Mais uma vez, muito feliz com o feedback do código @Roger-Melo . Pode realizar o fechamento da issue, estarei aplicando as sugestões ao código. Não havia reparado que usei event dentro da função que tinha como parâmetro o "e". Também não conhecia o bug ocasionado pela omissão dos blocos IF, me convencionei a não utilizá-lo quando a execução do código dentro da condicional não fosse maior do que 1 linha. Em casos maiores que 1 linha, como é o caso que o Feross apresentou no vídeo que mandou, o bug aconteceu porque existiu dois goto statements, certo? Existe alguma instância onde ocultar as chaves em condicionais de somente 1 linha é prejudicial? Além disso, por consistência e legibilidade, é melhor simplesmente colocar em todos?

Caso aqui não seja o lugar dessas perguntas, pode fechar a issue e me informar que abro uma outra de dúvida, Roger. Vou finalizar agradecendo mais uma vez, e dizendo que o seu curso realmente atinge o objetivo de fazer o aluno se sentir mais confiante no seu código. Isso, por si só, já é uma vantagem enorme. Muito obrigado, professor.

Muito obrigado, @danielscastro!

No que eu puder ajudar para acelerar sua chegada na fluência, conte comigo =)

Caso aqui não seja o lugar dessas perguntas, pode fechar a issue e me informar que abro uma outra de dúvida, Roger.

Aqui é o lugar certíssimo para tirar suas dúvidas sobre JS puro.

Não tem problema eu responder nesse comentário. Quem pesquisar pelo tema vai achar, mesmo que o título não seja especificamente sobre isso.

Existe alguma instância onde ocultar as chaves em condicionais de somente 1 linha é prejudicial? Além disso, por consistência e legibilidade, é melhor simplesmente colocar em todos?

Não.

Mas vc precisará ter a certeza que vc e/ou seu time não adicionará uma linha, e isso é complicado prever. Mesmo que vc esteja em um time habituado a fazer code-review (e que tenha essa regra do ESLint configurada), é um design de código fácil de falhar. Principalmente se há pessoas menos experientes na equipe.

if (true)
  console.log('olá')
  console.log('oi') // essa linha não está no if

Por isso, eu prefiro evitar a omissão de bloco em ifs, while, for e qualquer outra funcionalidade que a permita.

E sim, é uma sintaxe que deixa o código mais conciso mas impacta na legibilidade.

No parágrafo "This is perfectly valid code..." aqui no MDN vc tb pode ver o que eles recomendam.

Vc também pode dar uma olhada nessa resposta que tem o maior número de votos sobre o assunto no Stack Overflow.

Acredito que as referências acima possam expandir sua visão sobre o assunto, pra vc tomar decisões baseadas em seu contexto =)

No que precisar, é só abrir uma nova issue =)

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

Aplicação: Todo List #2638

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 features da linguagem que não foram mostradas nas aulas?

Link do repositório da aplicação

Maiores dificuldades durante a implementação

Menores dificuldades durante a implementação

Percebo que, por conta da falta de familiaridade e treino, eu tenho certa resistência ao ato de refatorar, bem como acho difícil algumas vezes. Estou me esforçando para mexer e alterar, às vezes de forma desnecessária, só pra treinar esse aspecto.

O parâmetro `e`

`searchTodos`

Possível refactoring para melhorar a legibilidade

Omissão de blocos em `if`s

Legibilidade em condicionais

`closest`

Caso aqui não seja o lugar dessas perguntas, pode fechar a issue e me informar que abro uma outra de dúvida, Roger.

Existe alguma instância onde ocultar as chaves em condicionais de somente 1 linha é prejudicial? Além disso, por consistência e legibilidade, é melhor simplesmente colocar em todos?

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

Aplicação: Todo List #2638

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 features da linguagem que não foram mostradas nas aulas?

Link do repositório da aplicação

Maiores dificuldades durante a implementação

Menores dificuldades durante a implementação

Percebo que, por conta da falta de familiaridade e treino, eu tenho certa resistência ao ato de refatorar, bem como acho difícil algumas vezes. Estou me esforçando para mexer e alterar, às vezes de forma desnecessária, só pra treinar esse aspecto.

O parâmetro e

searchTodos

Possível refactoring para melhorar a legibilidade

Omissão de blocos em ifs

Legibilidade em condicionais

closest

Caso aqui não seja o lugar dessas perguntas, pode fechar a issue e me informar que abro uma outra de dúvida, Roger.

Existe alguma instância onde ocultar as chaves em condicionais de somente 1 linha é prejudicial? Além disso, por consistência e legibilidade, é melhor simplesmente colocar em todos?

O parâmetro `e`

`searchTodos`

Omissão de blocos em `if`s

`closest`