Skip to content

PR de Correção#10

Open
iago-srm wants to merge 20 commits intocorrecaofrom
master
Open

PR de Correção#10
iago-srm wants to merge 20 commits intocorrecaofrom
master

Conversation

@iago-srm
Copy link

PR de Correção


export class Docente extends User {

public especialidade?: string[]
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

especialidades seria um nome melhor, no plural.



export class Estudante extends User{
public hobby?: string[]
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hobbies

Comment on lines +2 to +22
public "id": string
public "nome": string
public "email": string
public "data_nasc": string
public "turma_id": string

constructor(
id:string,
nome:string,
email:string,
data_nasc:string,
turma_id:string
)
{
console.log("Chamando o constructor da Classe User")
this.id = id,
this.nome = nome,
this.email = email,
this.data_nasc = data_nasc
this.turma_id = turma_id
}
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sobre estilo de código: a declaração dos atributos não precisa de aspas, e pode-se usar a notação constructor(public id: string, public nome: string, etc) {} para ter um código mais enxuto.
Também não deixar console.logs perdidos por aí.

Comment on lines +20 to +23
if (typeof(especialidade) != "object") {
res.status(errorCode).send("A(s) especialidade(s) deve(m) vir em forma de array!")
return
}
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Isso aqui não garante que a variável é uma array, pois ela pode ser um objeto e passar nessa verificação. Uma solução seria checar, também, se a propriedade length é diferente de undefined. Se for, vai ser uma array e não um objeto.

Comment on lines +38 to +42
if (buscaEspecialidade.length === 0) {
await connection("LS_Docente_Especialidade").delete().where("id_docente", "like", response[0].id)
await connection("LS_Docente").delete().where("email", "like", email)
res.send("Uma ou mais especialidades digitadas não existem.")
return
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Não seria melhor verificar se a especialidade é válida antes de ter inserido o docente? Assim não seria necessário removê-lo depois.

Copy link
Contributor

@labenu-bot labenu-bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Parabéns pela entrega!

Requisitos do projeto ✅

Implementações Feito
Tabela de turmas contendo id, nome e modulo
Tabela de estudantes contendo id, nome, email, data_nasc e turma_id (FK)
Tabela de docentes contendo id, nome, email, data_nasc e turma_id (FK)
Tabela de hobbies e tabela de junção (estudante/hobby)
Tabela de especialidades e tabela de junção (docente/especialidade)
Endpoint de criação de turma funcionando
Endpoint de 'busca de turmas ativas' e/ou 'mudança de módulo de turma' funcionando
Endpoint de criação de estudante funcionando
Endpoint de 'busca por nome de estudante' e/ou 'mudança de turma de estudante' funcionando
Endpoint de criação de docente funcionando
Endpoint de 'busca por todas as pessoas docentes' e/ou 'mudança de turma de docente' funcionando
[Desafio] Busca por todas as pessoas (estudantes e docentes) de uma mesma turma -
[Desafio] Busca por todas as pessoas com o mesmo hobby (estudantes) -
[Desafio] Busca por todas as pessoas com a especialidade 'POO' (docentes) -
[Desafio] Busca por todas as pessoas com o mesmo signo (estudantes e docentes) -
Feedback do código Feito

Comentários da pessoa avaliadora

Muito bom!
Todas as boas práticas ensinadas foram implementadas.
A organização das pastas ficou interessante, separando-as pelos métodos HTTP dentro de uma pasta de endpoints. Nunca tinha visto e achei legal.
Vejam mais comentários no PR!

Análise realizada por: Iago Soriano

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants